-
Notifications
You must be signed in to change notification settings - Fork 223
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
Add ClassSignature.self (fix #1568) #1617
Conversation
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.
Two minor comments, otherwise looks great!
@@ -810,8 +810,7 @@ example.Example.main().(args) => param args: Array[String] | |||
example.Example.x(). => val method x: ClassTag[Int] | |||
ClassTag => scala.reflect.ClassTag# | |||
Int => scala.Int# | |||
local0 => selfparam self: Example | |||
Example => example.Example. | |||
local0 => selfparam self: <?> |
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.
Why is this <?>
now?
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.
Because previously we were saving synthetic self-type assigned to selfparams by scalac. In this PR, I found a way to separate the synthetic part from what's actually written in the source code. Now, selfparams without explicit type information have no signature.
class C2 extends B { self: B => | ||
} | ||
|
||
class C3 extends B { self: B with Int => |
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.
Can we test that the name of the symbol is kept even when it's not self
? For example, class A { hello => }
?
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.
Done. The name is not kept in signatures though (because ScalaSignatures don't have the capacity to save it), only in occurrences.
For an explanation of the background, see scalameta#1618 (comment).
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.
LGTM 👍 Thanks for the explanation!
Based on top of #1616 to separate large-scale and non-large-scale changes to SemanticDB. This PR was surprisingly easy to implement.