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

[SemanticDB]Support new Scala3 modifiers in SemanticDB #13239

Merged

Conversation

tanishiking
Copy link
Member

Now we distinguish implicit and given in SemanticDB.

It might be better to wait merging until scalameta/scalameta#2439 has merged.

now we distinguish implicit and given in SemanticDB

- Proposal to add new properties to SemanticDB schema
  - scalameta/scalameta#2439
- Generated scala code copied from
  - tanishiking/semanticdb-for-scala3#2
@tanishiking tanishiking changed the title Support new Scala3 modifiers in SemanticDB [SemanticDB]Support new Scala3 modifiers in SemanticDB Aug 3, 2021
@@ -429,7 +429,7 @@ class ExtractSemanticDB extends Phase:
props |= SymbolInformation.Property.FINAL.value
if sym.is(Sealed) then
props |= SymbolInformation.Property.SEALED.value
if sym.isOneOf(GivenOrImplicit) then
Copy link
Member Author

@tanishiking tanishiking Aug 3, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be better to keep implicit property for givens for backward compatibility, and remove it may be in the next minor version release of Scala3?
(Or we can go because I suppose there are quite a few consumers for this information: probably only metals? and it's controllable)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tried that already, but it turned out that it was overall simplest the way it is now.

@@ -935,7 +935,7 @@ _empty_/Enums.Suits.Clubs. => case val static enum method Clubs
_empty_/Enums.Suits.Diamonds. => case val static enum method Diamonds
_empty_/Enums.Suits.Hearts. => case val static enum method Hearts
_empty_/Enums.Suits.Spades. => case val static enum method Spades
_empty_/Enums.Suits.derived$CanEqual. => implicit lazy val method derived$CanEqual
_empty_/Enums.Suits.derived$CanEqual. => lazy val given method derived$CanEqual
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this really given? Can they be lazy?

Suggested change
_empty_/Enums.Suits.derived$CanEqual. => lazy val given method derived$CanEqual
_empty_/Enums.Suits.derived$CanEqual. => lazy val given method derived$CanEqual

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, this is actually Given. This symbol is automatically generated given value by drives CanEqual

enum Suits derives CanEqual:
  case Hearts, Spades, Clubs, Diamonds

// `drives CanEuqual` will automatically generate given instance something like
lazy given val derived$CanEqual: CanEqual[Enums.Suits, Enums.Suits] = 
   CanEqual.derived

https://dotty.epfl.ch/docs/reference/contextual/derivation.html

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, parameterless givens are mapped to lazyvals by default. Some of them are mapped to defs when we can prove that that does not change semantics, so that we can omit generating a field.

Copy link
Contributor

@odersky odersky left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know the SemanticDB codebase. What concerns the embedding in the rest of the compiler it LGTM

@@ -429,7 +429,7 @@ class ExtractSemanticDB extends Phase:
props |= SymbolInformation.Property.FINAL.value
if sym.is(Sealed) then
props |= SymbolInformation.Property.SEALED.value
if sym.isOneOf(GivenOrImplicit) then
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We tried that already, but it turned out that it was overall simplest the way it is now.

@@ -935,7 +935,7 @@ _empty_/Enums.Suits.Clubs. => case val static enum method Clubs
_empty_/Enums.Suits.Diamonds. => case val static enum method Diamonds
_empty_/Enums.Suits.Hearts. => case val static enum method Hearts
_empty_/Enums.Suits.Spades. => case val static enum method Spades
_empty_/Enums.Suits.derived$CanEqual. => implicit lazy val method derived$CanEqual
_empty_/Enums.Suits.derived$CanEqual. => lazy val given method derived$CanEqual
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, parameterless givens are mapped to lazyvals by default. Some of them are mapped to defs when we can prove that that does not change semantics, so that we can omit generating a field.

@tgodzik tgodzik merged commit 03c2cb1 into scala:master Aug 6, 2021
@tanishiking tanishiking deleted the support-new-scala3-modifiers-semanticdb branch August 6, 2021 12:02
@Kordyjan Kordyjan added this to the 3.1.0 milestone Aug 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants