-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Create {And,Or}TypeTree in Parsers, not desugar #4424
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
Create {And,Or}TypeTree in Parsers, not desugar #4424
Conversation
The new API is required for all use cases in this PR.
Fix scala#4373. The testcase is from examples in scala#4373, variations and tests for types with wildcards. I'm unsure for a couple cases.
Without this commit, the testcase produces additional spurious errors: ```scala -- [E104] Syntax Error: tests/neg/i4373.scala:10:0 ----------------------------- 10 |class A2 extends _ with _ // error // error |^ |class Any is not a trait longer explanation available when compiling with `-explain` -- [E104] Syntax Error: tests/neg/i4373.scala:11:21 ---------------------------- 11 |class A3 extends Base with _ // error | ^ | class Any is not a trait longer explanation available when compiling with `-explain` -- Error: tests/neg/i4373.scala:12:24 ------------------------------------------ 12 |class A4 extends _ with Base // error | ^^^^ |illegal trait inheritance: superclass Any does not derive from trait Base's superclass Object ``` These errors make absolutely no sense for a user, since they refer to code the user didn't write, but that was generated by the compiler's error correction mechanism. I haven't checked if we could use `Ident(nme.ERROR)` everywhere. I have also tried using `EmptyTree` and `EmptyTree.withPos(wildcardPos)` and `EmptyTree.clone.withPos(wildcardPos)` as fallback trees, but all of them failed in some way.
TODO: make checkWildcard private to Parsers again.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As I wrote in my comment in the other thread, this loses the position of the operator. You can't hover over the operator and get its type or go to its definition anymore. So I don't think this is a good idea.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Edit: Since this is not a real operator it looks like we don't need its position after all. So, LGTM.
Essentially reviewed in scala#4424.
Essentially reviewed in scala#4424.
Essentially reviewed in scala#4424.
Refactoring on top of #4422, suggested by @smarter, WIP.
checkWildcard
private to Parsers again.