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

Reduce number of Symbol.tree dependencies in macros. #525

Conversation

WojciechMazur
Copy link
Contributor

@WojciechMazur WojciechMazur commented Apr 19, 2024

Symbol.tree usage is considered as an anti-pattern, because the tree is not guaranteed to exist.

Right now the only use case, that cannot be resolved without trees, is the annotation of base classes, eg. class X extends Y @MyAnnot.

This change was mostly motivated by the bug in the compiler I was not able to reproduce and minimise yet - under some conditions case class parameters can be generated before inlining (macros expansion) finishes. This leads to creation of param accessors that are not correctly handled in quotes. The call to CollectAnnotations.fromDeclarations with a parameter accessor would not be correctly handled. When execution Symbol.tree compiler would see that symbol has no Method flag so it assumes it as a ValDef, however, actually the tree of the parameter accessor is a DefDef which leads to MatchError when interpreting macros. Stack trace from this kind of errors can be found below

[error]  43 |  val x = summon[sttp.tapir.Schema[GroupsResponse]]
[error]     |                                                   ^
[error]     |  Exception occurred while executing macro expansion.
[error]     |  scala.MatchError: DefDef(groups,List(List()),TypeTree[TypeRef(ThisType(TypeRef(NoPrefix,module class immutable)),class List)],EmptyTree) (of class dotty.tools.dotc.ast.Trees$DefDef)
[error]     |          at dotty.tools.dotc.quoted.reflect.FromSymbol$.valDefFromSym(FromSymbol.scala:50)
[error]     |          at dotty.tools.dotc.quoted.reflect.FromSymbol$.definitionFromSym(FromSymbol.scala:22)
[error]     |          at scala.quoted.runtime.impl.QuotesImpl$reflect$SymbolMethods$.tree(QuotesImpl.scala:2607)
[error]     |          at scala.quoted.runtime.impl.QuotesImpl$reflect$SymbolMethods$.tree(QuotesImpl.scala:2607)
[error]     |          at magnolia1.Macro$CollectAnnotations$$anon$4.applyOrElse(macro.scala:251)
[error]     |          at scala.collection.immutable.List.collect(List.scala:267)
[error]     |          at magnolia1.Macro$CollectAnnotations.magnolia1$Macro$CollectAnnotations$$fromDeclarations(macro.scala:254)
[error]     |          at magnolia1.Macro$CollectAnnotations.paramAnns(macro.scala:220)
[error]     |          at magnolia1.Macro$.paramAnns(macro.scala:43)

…usecases is considered as an antipattern, becouse the tree is not quaranteed to exist
@KacperFKorban KacperFKorban merged commit 25ddaf9 into softwaremill:scala3 Apr 19, 2024
4 checks passed
@WojciechMazur WojciechMazur deleted the fix/reduce-dependency-on-symbol-tree branch April 19, 2024 20:35
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 this pull request may close these issues.

2 participants