-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Quotes reflect: Allow to return DefDef from a val symbol tree #22603
base: main
Are you sure you want to change the base?
Conversation
@prolativ Since you already looked at the issue, I though you might have been interested, but feel free to not do a review or unassign yourself if you feel uncomfortable with that/ don't have time. |
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.
Would one now get different results for .tree
with and without -Yretain-trees
(with the difference other than some tree vs no/empty tree)?
BTW as this area of code is changed we could fix the doc string, which doesn't render well in scala-doc currently
@@ -4060,9 +4060,10 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching => | |||
* | |||
* If this symbol `isClassDef` it will return `a `ClassDef`, |
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.
* If this symbol `isClassDef` it will return `a `ClassDef`, | |
* If this symbol `isClassDef` it will return a `ClassDef`, |
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.
Ah, good catch!
@@ -4060,9 +4060,10 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching => | |||
* | |||
* If this symbol `isClassDef` it will return `a `ClassDef`, | |||
* if this symbol `isTypeDef` it will return `a `TypeDef`, |
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.
* if this symbol `isTypeDef` it will return `a `TypeDef`, | |
* if this symbol `isTypeDef` it will return a `TypeDef`, |
@@ -4060,9 +4060,10 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching => | |||
* | |||
* If this symbol `isClassDef` it will return `a `ClassDef`, | |||
* if this symbol `isTypeDef` it will return `a `TypeDef`, | |||
* if this symbol `isValDef` it will return `a `ValDef`, | |||
* if this symbol `isDefDef` it will return `a `DefDef` |
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.
* if this symbol `isDefDef` it will return `a `DefDef` | |
* if this symbol `isDefDef` it will return a `DefDef` |
@@ -4060,9 +4060,10 @@ trait Quotes { self: runtime.QuoteUnpickler & runtime.QuoteMatching => | |||
* | |||
* If this symbol `isClassDef` it will return `a `ClassDef`, | |||
* if this symbol `isTypeDef` it will return `a `TypeDef`, | |||
* if this symbol `isValDef` it will return `a `ValDef`, | |||
* if this symbol `isDefDef` it will return `a `DefDef` | |||
* if this symbol `isBind` it will return `a `Bind`, |
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.
* if this symbol `isBind` it will return `a `Bind`, | |
* if this symbol `isBind` it will return a `Bind`, |
* if this symbol `isDefDef` it will return `a `DefDef` | ||
* if this symbol `isBind` it will return `a `Bind`, | ||
* if this symbol `isValDef` it will return `a `ValDef`, |
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.
* if this symbol `isValDef` it will return `a `ValDef`, | |
* if this symbol `isValDef` it will return a `ValDef`, |
Yes, they would be different:
This means that the .tree can even return different things in different incremental compilation runs of the same code. |
Ideally we would be able to turn back time and always be able to see what the tree looked like in Pickler and use that, but in general even then there would be inconsistencies like these when |
I had to remove
@oxygen.meta.annotation.showDerivation
final case class CaseClass1(
@ExampleTypeClasses.fieldName("my-int") int: Int,
@ExampleTypeClasses.misc string: String,
boolean: Boolean,
)
@oxygen.meta.annotation.showDerivation final case class CaseClass1(int: scala.Int, string: scala.Predef.String, boolean: scala.Boolean) extends java.lang.Object with _root_.scala.Product with java.io.Serializable {
override def hashCode(): scala.Int
override def equals(x$0: scala.Any): scala.Boolean
override def toString(): java.lang.String
override def canEqual(that: scala.Any): scala.Boolean
override def productArity: scala.Int
override def productPrefix: scala.Predef.String
override def productElement(n: scala.Int): scala.Any
} |
This is expected and explicitly documented for the |
seems annotations are only available on the constructor symbols, not the case fields or declared fields, interesting |
Fixes #22584
During the
Getters
phase, the compiler replaces class member vals with defs. Meanwhile.tree
returns a tree most recently associated with a symbol (if currently compiled), or one read from tasty (if compiled before). An interesting thing happens in the issue - since all of the files are tried being compiled in one compilation run, the file with a macro call is suspended, and then the macro and the MyClass class definition are compiled to the end (also including theGetters
phase, where the tree stops being a ValDef).Initially I thought about manually replacing that returned DefDef with a ValDef in quotes reflection (like what happens when the tree is not available), but that would have been pointless - there would be little use of calling
.tree
on a symbol if we cannot read the rhs of the definition. Returning raw DefDef seems like a less bad choice, especially.tree
is meant to be for power users anyway, and is full of other, more dangerous warts.