Fix #9751: Fix Inline purity checks #9753
Conversation
nicolasstucki
force-pushed
the
fix-#9751
branch
from
5f8f19e to
d97fc04
Compare
nicolasstucki
changed the title
nicolasstucki
force-pushed
the
fix-#9751
branch
from
d97fc04 to
bf64e9d
Compare
nicolasstucki
changed the title
nicolasstucki
force-pushed
the
fix-#9751
branch
from
bf64e9d to
9b3d80d
Compare
We wrongly identified most inline references as pure as they are effectively erased. Only erased terms are always pure. Most of the purity of inline references follows from the normal purity rules except for inline parameters. These parameters lose their binding when inlined and no purity guaranty can be ensured from the reference.
nicolasstucki
force-pushed
the
fix-#9751
branch
from
9b3d80d to
c159361
Compare
| && !ctx.isAfterTyper | ||
| && !tree.isInstanceOf[Inlined] | ||
| && isPureExpr(tree) | ||
| && !isSelfOrSuperConstrCall(tree) |
There was a problem hiding this comment.
tree.isInstanceOf[Inlined] : can an inlined tree be pure? What about handle inlined trees in isPureExpr?
There was a problem hiding this comment.
The Inlined trees can be pure which is correctly identified in isPureExpr. Here we special case a
Inlined calls that inserted a pure expression in a statement position as these are only pure due to other constraints that may change. For example, the assert can take a reference to an Inlined true value which converts it in a no-op and we do not want to warn that this is a pure expression in statement position.
There was a problem hiding this comment.
Thanks for the explanation. It might be good to add a short comment after the line.
| @@ -47,7 +47,7 @@ abstract class TransformByNameApply extends MiniPhase { thisPhase: DenotTransfor | |||
| ref(defn.cbnArg).appliedToType(argType).appliedTo(arg).withSpan(arg.span) | |||
| arg match { | |||
| case Apply(Select(qual, nme.apply), Nil) | |||
| if qual.tpe.derivesFrom(defn.FunctionClass(0)) && isPureExpr(qual) => | |||
| if qual.tpe.derivesFrom(defn.FunctionClass(0)) && (isPureExpr(qual) || qual.symbol.isAllOf(Inline | Param)) => | |||
There was a problem hiding this comment.
Does it matter? If I were correct, the body of inlined methods is never executed, thus only the TASTY matters.
There was a problem hiding this comment.
In theory that seems to be the case. But I caught one that came for the DottyPredef.assert when compiling dotty itself. It might be a re-bootstrapping issue.
There was a problem hiding this comment.
I see. It could be some invariants after the typer -- and it's always good to do so.
We wrongly identified most inline references as pure as they are effectively erased.
Only erased terms are always pure.
Most of the purity of inline references follows from the normal purity rules except for inline parameters.
These parameters lose their binding when inlined and no purity guaranty can be ensured from the reference.