Avoid leak of internal implementation in tasty.Reflection #9613
Conversation
nicolasstucki
force-pushed
the
link-quoted-and-tasty-context
branch
2 times, most recently
from
ba5d236 to
b6d0c5b
Compare
|
Note that this change does not affect the |
compiler/src/dotty/tools/dotc/tastyreflect/ReflectionCompilerInterface.scala
Outdated
Show resolved
Hide resolved
compiler/src/dotty/tools/dotc/tastyreflect/ReflectionImpl.scala
Outdated
Show resolved
Hide resolved
| */ | ||
| def betaReduce[T](expr: Expr[T])(using qctx: QuoteContext): Expr[T] = | ||
| qctx.tasty.internal.betaReduce(expr.unseal) match | ||
| val qctx2 = scala.internal.tasty.CompilerInterface.leaked(qctx) |
There was a problem hiding this comment.
The name leaked is not intuitive about what it does? It seems better to use an more informative verb instead.
There was a problem hiding this comment.
It is not obvious why we cannot just use qctx here, as before?
There was a problem hiding this comment.
The trick is that the public API of QuoteContext exposes QuoteContext.tasty as Reflection, but we always implement is by mixing in the CompilerInterface. These methods are not intended to be used directly by the users, so we hide them inside the CompilerInterface. This interface should only be used within the library.
There was a problem hiding this comment.
I renamed it to quoteContextWithCompilerInterface
There was a problem hiding this comment.
What about just return an instance of CompilerInterface? That will make things more clean:
val ci: CompilerInterface = scala.internal.tasty.CompilerInterface(qtx)There was a problem hiding this comment.
That was what I tried.
[error] -- [E007] Type Mismatch Error: /Users/nicolasstucki/GitHub/dotty2/library/src-bootstrapped/scala/quoted/Expr.scala:80:18
[error] val ci = CompilerInterface(qctx)
[error] 80 | ci.betaReduce(expr.unseal) match
[error] | ^^^^^^^^^^^
[error] | Found: qctx.tasty.Term
[error] | Required: ci.TermThere was a problem hiding this comment.
Might be a bug in TypeComparer? I'll try to minimize.
There was a problem hiding this comment.
We can improve that that later. This change only affects internal implementation.
There was a problem hiding this comment.
@liufengyun Dotty currently doesn't support reasoning about refining singletons, which feels like a big oversight: lampepfl/dotty-feature-requests#89
There was a problem hiding this comment.
Thanks for the pointer @LPTK . Yes, indeed it is. We also lose the reasoning about paths for class parameters. I agree that a systematic approach is needed to support reasoning about path equality in path-dependent types.
| val argTypes = functionType.unseal.tpe match | ||
| case AppliedType(_, functionArguments) => functionArguments.init.asInstanceOf[List[Type]] | ||
| qctx.tasty.internal.lambdaExtractor(expr.unseal, argTypes).map { fn => | ||
| val qctx2 = scala.internal.tasty.CompilerInterface.leaked(qctx) |
There was a problem hiding this comment.
Same question as above: can we just use qctx?
|
|
||
| def unseal(using qctx: QuoteContext): qctx.tasty.Term = | ||
| if (qctx.tasty.internal.compilerId != scopeId) | ||
| if (scala.internal.tasty.CompilerInterface.leaked(qctx).tasty.compilerId != scopeId) |
| /** View this expression `quoted.Type[T]` as a `TypeTree` */ | ||
| def unseal(using qctx: QuoteContext): qctx.tasty.TypeTree = | ||
| if (qctx.tasty.internal.compilerId != scopeId) | ||
| if (scala.internal.tasty.CompilerInterface.leaked(qctx).tasty.compilerId != scopeId) |
* Remove `Reflection.internal`: the source of the leak * Add `CompilerInterface` as a self type * Add common type abstraction for `Reflection` and `ComplerInterface` * Add `ComplerInterface.leak` to allow access to internals within the library
nicolasstucki
force-pushed
the
link-quoted-and-tasty-context
branch
from
b6d0c5b to
a2d3821
Compare
3 participants
Reflection.internal: the source of the leakCompilerInterfaceas a self typeReflectionandComplerInterfaceComplerInterface.leakto allow access to internals within the library