Use tree checker in macros #16570
Use tree checker in macros #16570
Conversation
nicolasstucki
force-pushed
the
use-tree-checker-in-macros
branch
from
bca7cc4 to
b3b7889
Compare
nicolasstucki
force-pushed
the
use-tree-checker-in-macros
branch
3 times, most recently
from
2ef4a2d to
d9a1b24
Compare
nicolasstucki
force-pushed
the
use-tree-checker-in-macros
branch
3 times, most recently
from
3c7011b to
c50ef24
Compare
|
We should test this on the extended community build before merging. |
|
Extended community test: https://scala3.westeurope.cloudapp.azure.com/job/runBuild/125/ |
| if ctx.settings.XcheckMacros.value then | ||
| val checkingCtx = ctx | ||
| .fresh | ||
| .addMode(Mode.ImplicitsEnabled) |
There was a problem hiding this comment.
Do we really need to generate implicits when checking trees?
There was a problem hiding this comment.
I followed the original tree checker https://github.com/lampepfl/dotty/blob/main/compiler/src/dotty/tools/dotc/transform/TreeChecker.scala#L137.
I will try removing both.
| val implicitTree = ctx.typer.inferImplicitArg(tpe, Position.ofMacroExpansion.span) | ||
| // Make sure that we do not have any uninstantiated type variables. | ||
| // See tests/pos-macros/exprSummonWithTypeVar with -Xcheck-macros. | ||
| implicitTree.foreachSubTree(tree => dotc.typer.Inferencing.fullyDefinedType(tree.tpe, "", tree)) |
There was a problem hiding this comment.
foreachSubTree is expensive. You could use typer.interpolateTypeVars instead (probably with pt = WildcardType and locked = ctx.typerState.ownedVars).
There was a problem hiding this comment.
interpolateTypeVars did not work.
There was a problem hiding this comment.
I there a way to make inferImplicitArg resolve all type variables that it inserts?
There was a problem hiding this comment.
Try defining val locked = ctx.typerState.ownedVars before the call to inferImplicitArg, so that variables created by it can be instantiated when you pass locked to interpolateTypeVars
There was a problem hiding this comment.
It did not work either. Note that the ownedVars of this context are empty before and after the call to inferImplicitArg. The type variable in introduced within inferImplicitArg.
nicolasstucki
force-pushed
the
use-tree-checker-in-macros
branch
from
4fe1657 to
9cea198
Compare
The issue was that an uninstantiated type variable escaped the implicit search and was then affected by the quote pattern match. Fixes scala#15779 Fixes scala#16636
Trees are only checked if -Xcheck-macros is enabled.
Fixes:
- Add missing positions to {ValDef,Bind}.apply
- Inline by-name ascribed param
- Buggy test (tests/neg-macros/annot-mod-top-method-add-top-method)
nicolasstucki
force-pushed
the
use-tree-checker-in-macros
branch
from
be3ce46 to
aaf86b3
Compare
| @@ -9,5 +9,5 @@ class addTopLevelMethodOutsidePackageObject extends MacroAnnotation: | |||
| import quotes.reflect._ | |||
| val methType = MethodType(Nil)(_ => Nil, _ => TypeRepr.of[Int]) | |||
| val methSym = Symbol.newMethod(Symbol.spliceOwner.owner, Symbol.freshName("toLevelMethod"), methType, Flags.EmptyFlags, Symbol.noSymbol) | |||
| val methDef = ValDef(methSym, Some(Literal(IntConstant(1)))) | |||
| val methDef = DefDef(methSym, _ => Some(Literal(IntConstant(1)))) | |||
There was a problem hiding this comment.
After rebasing, the check caught this unintended bug in this neg test.
Trees are only checked if
-Xcheck-macrosis enabled.Fixes:
{ValDef,Bind}.applyobjectafter a seemingly innocent quote match #16636