-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Avoid loosing denotations of named types during integrate
#22839
base: main
Are you sure you want to change the base?
Conversation
…`TypeMap`s" This reverts commit bb63e31.
Fixes #22874. `wildApprox` approximates parameter references and type variables by wildcards. When doing so for an `AnnotatedType`, this can produce trees with wildcards types, causing the type assigner to fail. For example, consider `Apply(fn, args)` where `fn` has type `TermParamRef`. Applying `wildApprox` will approximate the type of `fn` to a wildcard, leading [the type assigner for `Apply`](https://github.com/scala/scala3/blob/cb97c40930d335e0fca38238682d218c3e718bd8/compiler/src/dotty/tools/dotc/typer/TypeAssigner.scala#L298) to emit an error stating that `<?>` does not take parameters. This issue is somehow similar to the one described in #19957 (comment), which was fixed by #21941 (and re-worked in #22839). This PR fixes the issue by approximating annotated types in `wildApprox`: annotated types are approximated by their parent types if they are not refining, or by wildcards upper-bounded by their parent types if they are.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Otherwise LGTM
private class IntegrateMap(params: List[Symbol], paramRefs: List[Type])(using Context) extends TypeMap: | ||
override def apply(tp: Type) = | ||
tp match | ||
case tp: NamedType if params.contains(tp.symbol) => paramRefs(params.indexOf(tp.symbol)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is duplicates code from SubstMap, but at lower performance. I think we should factor out only what is essential here:
-
Make Substituters.SubstMap non-final and note that it is overridden by IntergrateMap in the comment.
-
Define IntegrateMap as
private class IntegrateMap(params: List[Symbol], paramRefs: List[Type])(using Context) extends Substituters.SubstMap(params, paramrefs): override def derivedSelect(tp: NamedType, pre: Type): Type = ...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As discussed orally, we can't extend SubstMap
, because it uses subst
which directly calls Type#derivedSelect
, not TypeMap#derivedSelect
:
else tp.derivedSelect(subst(tp.prefix, from, to, theMap)) |
I instead inlined the code from SubstMap
as you suggested, done in 9bef68c.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As a side-effect, that also restores the original behavior for the test i12448
, which my previous implementation changed:
Lines 1 to 5 in 7976598
object Main { | |
def mkArray[T <: A]: T#AType // error // error | |
mkArray[Array] // was: "assertion failed: invalid prefix HKTypeLambda..." // error | |
val x = mkArray[Array] // error | |
} |
This is the output with the SubstMap
implementation:
sbt:scala3> scalac tests/neg/i12448.scala
...
-- [E006] Not Found Error: tests/neg/i12448.scala:2:19 -------------------------
2 | def mkArray[T <: A]: T#AType // error // error
| ^
| Not found: type A
|
| longer explanation available when compiling with `-explain`
-- [E067] Syntax Error: tests/neg/i12448.scala:2:6 -----------------------------
2 | def mkArray[T <: A]: T#AType // error // error
| ^
|Declaration of method mkArray not allowed here: only classes can have declared but undefined members
-- Error: tests/neg/i12448.scala:3:9 -------------------------------------------
3 | mkArray[Array] // was: "assertion failed: invalid prefix HKTypeLambda..." // error
| ^
| invalid new prefix Array cannot replace T in type T#AType
-- Error: tests/neg/i12448.scala:4:17 ------------------------------------------
4 | val x = mkArray[Array] // error
| ^
| invalid new prefix Array cannot replace T in type T#AType
4 errors found
while with my previous implementation, the two invalid prefix
errors did not appear.
No description provided.