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

Open
vicuna opened this Issue Feb 11, 2019 · 4 comments

Comments

Projects
None yet
2 participants
@vicuna
Copy link
Collaborator

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

This comment has been minimized.

Copy link
Collaborator 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

This comment has been minimized.

Copy link
Collaborator Author

vicuna commented Feb 11, 2019

Comment author: @diml

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

@vicuna

This comment has been minimized.

Copy link
Collaborator 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

This comment has been minimized.

Copy link
Collaborator 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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.