[Scala3] Add Instantiate API#5278
Conversation
| val sourceInfoExpr = Expr.summon[SourceInfo].getOrElse { | ||
| report.errorAndAbort("Instantiate.apply requires an implicit SourceInfo") | ||
| } |
There was a problem hiding this comment.
Can we just thread this through the user API instead so the user can pass it manually if they want?
|
|
||
| def definitionMacro[A <: BaseModule: Type](con: Expr[A])(using q: Quotes): Expr[core.Definition[A]] = { | ||
| import q.reflect.* | ||
| val (argsTuple, constructorFunc) = extractAndBuild[A](con) |
There was a problem hiding this comment.
Why does instanceMacro have a source locator but this doesn't?
| private object InstantiateIntfMacros { | ||
| import scala.quoted.* | ||
|
|
||
| private def extractAndBuild[A <: BaseModule: Type]( |
There was a problem hiding this comment.
Can you add a comment describing what this returns? It appears to be the arguments as a tuple and then the constructor function value, but it'd be good to have that in English.
| con: Expr[A] | ||
| )(using q: Quotes): (Expr[Any], Expr[_]) = { | ||
| import q.reflect.* | ||
| def unwrapInlined(term: Term): Term = term match { |
There was a problem hiding this comment.
Comment describing would be good
| val isImplicit = inner.tpe.widen match { | ||
| case mt: MethodType => mt.isImplicit | ||
| case _ => false | ||
| } | ||
|
|
||
| if (isImplicit) { | ||
| collectArgs(inner, accFlat, accStructured) | ||
| } else { | ||
| collectArgs(inner, newArgs ::: accFlat, newArgs :: accStructured) | ||
| } |
There was a problem hiding this comment.
Are we dropping implicit arguments? That does not seem right
| case Select(New(tpt), _) => | ||
| (tpt, None, unwrapped) | ||
| case TypeApply(Select(New(tpt), _), targs) => | ||
| (tpt, Some(targs), unwrapped) |
There was a problem hiding this comment.
What are these two cases?
| } | ||
| ).asExpr | ||
| } | ||
|
|
There was a problem hiding this comment.
I find the mix of vals and defs hard to follow, can you collect all of the defs separately from the vals? Maybe as separate private defs outside of extractAndBuild or at least at the top of the function?
| case List(t1, t2) => | ||
| AppliedType(TypeRepr.of[Tuple2].typeSymbol.typeRef, List(t1, t2)) | ||
| case List(t1, t2, t3) => | ||
| AppliedType(TypeRepr.of[Tuple3].typeSymbol.typeRef, List(t1, t2, t3)) | ||
| case _ => | ||
| AppliedType(defn.TupleClass(n).typeRef, argTypes) |
There was a problem hiding this comment.
Is it standard to have to handle tuple 2 and tuple 3 specially? If so please add a comment.
| buildConstructorLambdaMultiList( | ||
| List(tupleTypeRepr), | ||
| params => { | ||
| val flatArgs = (1 to n).map(i => Select.unique(params.head.asInstanceOf[Term], s"_$i")).toList |
There was a problem hiding this comment.
The repeated use of params.head.asInstanceOf[Term] looks like a bug, I'm assuming it isn't. Can you hoist it out of the map and give it a meaningful name and possibly comment describing what it is?
adkian-sifive
force-pushed
the
adkian-sifive/scala3-instantiate
branch
from
99b0784 to
26fc1c9
Compare
2 participants
Contributor Checklist
docs/src?Type of Improvement
Desired Merge Strategy
Release Notes
Reviewer Checklist (only modified by reviewer)
3.6.x,5.x, or6.xdepending on impact, API modification or big change:7.0)?Enable auto-merge (squash)and clean up the commit message.Create a merge commit.