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

Surprising priority of algebraic attributes #7916

Closed
vicuna opened this issue Feb 11, 2019 · 6 comments
Closed

Surprising priority of algebraic attributes #7916

vicuna opened this issue Feb 11, 2019 · 6 comments

Comments

@vicuna
Copy link

vicuna commented Feb 11, 2019

Original bug ID: 7916
Reporter: AltGr
Assigned to: @diml
Status: assigned (set by @diml on 2019-02-11T13:51:57Z)
Resolution: open
Priority: low
Severity: minor
Version: 4.07.1
Category: lexing and parsing
Monitored by: @nojb @diml

Bug description

The manual is not very clear about the priority of the postfix [@foo ] notation, esp. w.r.t. infix operators. The parser defines it between :: and INFIXOP1 (@, ^), and in fact, the presence of attributes can change the meaning of an expression, which I find surprising.

There may be good reasons for this though ?

Steps to reproduce

# 3 + 2 * 4;;
- : int = 11
# 3 + 2 [@foo] * 4;;
- : int = 20

Additional information

Found this while debugging ocp-indent issue #275, OCamlPro/ocp-indent#275

@vicuna
Copy link
Author

vicuna commented Feb 11, 2019

Comment author: @diml

I remember changing some of the priorities of attribute attachment. I'll try to find the discussions, but the original motivation was to make the attachment of annotations in record fields and constructors more natural. In the end we decided to make the rules consistent between expressions, types, etc...

@vicuna
Copy link
Author

vicuna commented Feb 11, 2019

Comment author: @diml

It was this GPR: #152
which followed this mantis ticket: #6612

@vicuna
Copy link
Author

vicuna commented Feb 11, 2019

Comment author: AltGr

Ah, indeed I understand the motivation better. In the GPR, only the effects on type definitions are considered, though (and those make sense!)
My issue is the result on the parsing of expressions, though.

@vicuna
Copy link
Author

vicuna commented Feb 12, 2019

Comment author: @diml

That's true. And the PR actually doesn't change the parsing of expressions. We might have changed the parsing of expressions in a subsequent GPR in order to make everything more homogeneous.

Maybe the rule we used wasn't the best one, and instead it could have been: attach the attribute to the biggest syntactic element possible without changing the parsing. i.e. erasing attributes in the source code should yield the same AST modulo attributes.

It's probably quite a lot of work to change that now though.

@vicuna vicuna assigned ghost Mar 14, 2019
@vicuna vicuna added the bug label Mar 20, 2019
AltGr added a commit to OCamlPro/ocp-indent that referenced this issue Sep 23, 2019
Although it doesn't match the apparent priority given by the OCaml
parser, it's closer to what happens in practice.

See ocaml/ocaml#7916 for details
@github-actions
Copy link

github-actions bot commented May 6, 2020

This issue has been open one year with no activity. Consequently, it is being marked with the "stale" label. What this means is that the issue will be automatically closed in 30 days unless more comments are added or the "stale" label is removed. Comments that provide new information on the issue are especially welcome: is it still reproducible? did it appear in other contexts? how critical is it? etc.

@github-actions github-actions bot added the Stale label May 6, 2020
@ghost
Copy link

ghost commented May 6, 2020

It's unlike to change at this point, so closing this issue.

@ghost ghost closed this as completed May 6, 2020
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant