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

Bug: ambiguous + detection for Trait and lifetime bounds is broken #285

Closed
ruabmbua opened this issue Dec 17, 2018 · 3 comments · Fixed by #6160
Closed

Bug: ambiguous + detection for Trait and lifetime bounds is broken #285

ruabmbua opened this issue Dec 17, 2018 · 3 comments · Fixed by #6160

Comments

@ruabmbua
Copy link
Contributor

Example: playground
Related: issue #279

There are multiple cases, where the ambiguity of + is not detected. Note, that the analysis is inside libsyntax (e.g. rustfmt throws the ambiguous + error in all cases). This leads to code being able to pass the syntax check, and yielding incorrect results.

List of cases, where additional bounds via + are invalid (only the ones I could find / did know):

  • shared & mutable reference types
  • const and mutable raw pointer types
  • Closure return type position (these are handled wrong in libsyntax according to language spec)
  • bare function return type position (I think libsyntax got that one right)
  • and of course the type cast expression, which is mentioned in issue Bug: operator precedence around **as** keyword is wrong #279 (the fix should probably be replaced by a more general solution)

This should be solved by properly implementing TypeNoBounds as a sub parser for all type expressions.

TypeNoBounds can then be invoked in all of these 5 cases. Note, that the allow_bound mechanism in the path_() parser should probably be replaced by something else. E.g. by using TypeParamBounds in all the correct places and implementing it correctly.

@ruabmbua ruabmbua changed the title Ambiguous + detection for Trait and lifetime bounds is broken Bug: ambiguous + detection for Trait and lifetime bounds is broken Dec 17, 2018
@ruabmbua
Copy link
Contributor Author

I will try to create a PR, but this might be a little bit over my head.

@matklad
Copy link
Member

matklad commented Dec 18, 2018

@ruabmbua I think the best way to fix this might be not in the parser itself, but in the validation layer. That is, after the parsig is done, you traverse the tree and check that all disambiguating parenthesis are present.

@ruabmbua
Copy link
Contributor Author

A second pass seems like a good idea.

Maybe I can find some time to test this strategy.

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 a pull request may close this issue.

2 participants