Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Document abstract format of type-related trees #230

Closed
wants to merge 2 commits into from

Conversation

nox
Copy link
Contributor

@nox nox commented Feb 8, 2014

No description provided.

@OTP-Maintainer
Copy link

Patch has passed first testings and has been assigned to be reviewed

@proxyles
Copy link
Contributor

proxyles commented Jul 2, 2014

Hi,

We still have not decided when to include documentation of types and
specs in the abstract format.

When reviewing the patch we found several issues that have now been
fixed in 18.0 (be46e45). In particular there is a new tag, user_type.

A few minor issues/questions/suggestions follow.

opaque or type attribute. A_i is a "type variable". A type variable is
like a variable except that _ is not a type variable (_ is a name
for the any() type).

spec or callback. Callback Mod:F is unsupported (sys_pre_expand no
longer crashes in 18.0).

Type clauses. "unbounded fun type clause" should be replaced by just
"fun type clause" everywhere, or else it should be stated that a "fun
type clause" is the same as an "unbounded fun type clause".

Type guards. There should be a def of "type guard sequence", a
non-empty sequence of type guards separated by commas (similar to the
def of a guard G in section 6.6).

"If G is a type definition...". This is also a constraint, not a
definition. Name is a "type variable". The linter does not allow _
in 18.0 (Dialyzer has never liked it).

Types. "If G is a type definition Name :: Type." Suggestion: "If G is
an annotated type Annotation :: Type where Annotation is a variable."
Annotation can be _, but it is unclear what the purpose would be.

"If T is a variable V". Should be "is a type variable". Maybe there
should be a separate clause for _, the common alias for the any()
type.

map_field_assoc. Before be46e45 this was a five-tuple {type, LINE,
map_field_assoc, Rep(K), Rep(V)}, but what is said here is correct in
18.0 (no change needed).

"record field type <<>>". Should be "binary type".

<<_:B, :*U>>. Unless Kostis thinks otherwise the Reference Manual
should probably be updated to use "B" and "U" rather than "M" and "N"
(no change needed).

fun((...) -> Ret). Seems to be wrong.

Record fields. "does not contain undefined syntactically". Unclear
what it means; would it be possible to be more specific?

@nox
Copy link
Contributor Author

nox commented Jul 2, 2014

Thanks for the review, will look at your remarks later.

It was put back by error on a conflict hiccup when introducing named funs.
@nox
Copy link
Contributor Author

nox commented Aug 17, 2014

opaque or type attribute. A_i is a "type variable". A type variable is like a variable except that _ is not a type variable (_ is a name for the any() type).

I am not sure what to make of this comment, given that nowhere it is said that _ is a variable. The underscore is only mentioned under patterns, as the universal pattern.

@nox
Copy link
Contributor Author

nox commented Aug 17, 2014

But I did forget to document the _ alias for any(), though.

@nox
Copy link
Contributor Author

nox commented Aug 17, 2014

spec or callback. Callback Mod:F is unsupported (sys_pre_expand no longer crashes in 18.0).

It is indeed unsupported, but rules are too and they are still documented there. I don't think it's a good thing to not mention them.

@nox
Copy link
Contributor Author

nox commented Aug 17, 2014

Types. "If G is a type definition Name :: Type." Suggestion: "If G is an annotated type Annotation :: Type where Annotation is a variable." Annotation can be _, but it is unclear what the purpose would be.

Why would you not call it a type definition? It defines something that can be exported, like a function definition.

@nox
Copy link
Contributor Author

nox commented Aug 17, 2014

Record fields. "does not contain undefined syntactically". Unclear what it means; would it be possible to be more specific?

I meant that it is not a type union that explicitly includes 'undefined' as part of its alternatives. I find mentioning that it needs to be a syntactic mention the clearest way to convey this. Could you suggest an alternate wording?

@nox
Copy link
Contributor Author

nox commented Aug 17, 2014

I amended following some of your remarks and commented the others.

@proxyles
Copy link
Contributor

proxyles commented Sep 9, 2014

Hi,

spec or callback. Callback Mod:F is unsupported (sys_pre_expand no
longer crashes in 18.0).

It is indeed unsupported, but rules are too and they are still
documented there. I don't think it's a good thing to not mention them.

I don't follow your argument. Why should we document something that is
not supported?

I cannot see where rules (':-') are documented. If they are documented
I think it's due to a mistake. (Seems they are still supported by the
parser, though. Thought all support for Mnemosyne was removed a long
time ago...)

Type clauses. "unbounded fun type clause" should be replaced by just
"fun type clause" everywhere, or else it should be stated that a "fun
type clause" is the same as an "unbounded fun type clause".

I see you removed all occurrences of "bounded". That was not what I
meant... It think it should be "If T is a bounded fun type clause Tc
when Tg...". That way it's easy to see the correspondence between this
document and the refman.

Type guards. There should be a def of "type guard sequence", a
non-empty sequence of type guards separated by commas (similar to the
def of a guard G in section 6.6).

I see you've added a definition of type guard sequence. Using the name
"Gs" may be confusing as it is used for guard sequences. Maybe "TGs"?
The name should be used in the preceding def of bounded fun type, "Tc
when TGs" ("TGs" instead of "Tg").

The refman uses "constraints" rather than "type guard sequence". It
could be a good idea to follow the refman in order not to introduce
terms that are used in this document only. But I think we can keep
"type guard sequence", at least for now.

Types. "If G is a type definition Name :: Type."

Why would you not call it a type definition? It defines something that
can be exported, like a function definition.

Note that my comment was about the second clause of "Type guards", not
about the first clause of "Types". It looks OK now, I think.

Record fields. "does not contain undefined syntactically". Unclear
what it means; would it be possible to be more specific?

I meant that it is not a type union that explicitly includes
'undefined' as part of its alternatives. I find mentioning that it
needs to be a syntactic mention the clearest way to convey this. Could
you suggest an alternate wording?

"T is type that is not a type union that explicitly includes
'undefined' as part of its alternatives" is quite readable, I think.

Since the patch is indented for 18.0 (if we decide to include it...)
it would be nice if you could document 'user_type' ('type' is used for
predefined types, and 'user_type' is used for user-defined types).

Best regards

@nox
Copy link
Contributor Author

nox commented Sep 9, 2014

I see you removed all occurrences of "bounded". That was not what I
meant... It think it should be "If T is a bounded fun type clause Tc
when Tg...". That way it's easy to see the correspondence between this
document and the refman.

I disagree, it's weird to have both "fun type clause" and "bounded fun type clause", either qualify both or qualify none.

"T is type that is not a type union that explicitly includes
'undefined' as part of its alternatives" is quite readable, I think.

I disagree, there is no mention of syntax in this, one could believe that referencing foo:bar() "explicitly includes 'undefined'".

Since the patch is indented for 18.0 (if we decide to include it...)
it would be nice if you could document 'user_type' ('type' is used for
predefined types, and 'user_type' is used for user-defined types).

I have no idea what you are talking about with user_type, but I sure do want that patch to be included in 18.0, because I think it's long overdue that we document that part of Erlang.

@essen
Copy link
Contributor

essen commented Sep 9, 2014

About :-, I wanted to remove it from the parser entirely when I removed the query keyword but Robert told me he wanted to use them for his prolog thing so I left it there.

@uabboli
Copy link
Contributor

uabboli commented Sep 10, 2014

About :-, I wanted to remove it from the parser entirely when I removed the query keyword but
Robert told me he wanted to use them for his prolog thing so I left it there.

Thanks for the info. Robert just told me that he doesn't need ':-' for erlog, so ':-' can be removed from the scanner as well as the parser (in 18.0).

@OTP-Maintainer
Copy link

Patch has passed first testings and has been assigned to be reviewed


I am a script, I am not human


@nox
Copy link
Contributor Author

nox commented Nov 1, 2014

Ping?

@uabboli
Copy link
Contributor

uabboli commented Jun 12, 2015

Merged to master, commit 8e49f7 (it will take a few days for the commit to appear on Github).

Formatting of types in Syntax Tools has been put off due to lack of time.

@uabboli uabboli closed this Jun 12, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants