Skip to content

Fix cycle detection for type aliases with wildcard arguments #15508

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

Merged
merged 3 commits into from
Jun 23, 2022

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Jun 23, 2022

#15507 escaped the cycle detector since when the right hand side was typechecked,
the type constructor _NestedSet2's type was [X] <: Any and the constructor was marked Provisional.
When faced with the application NestedSet2[?], the type checker tried to reduce the application, since
abstract types like NestedSet2 are not allowed to take wildcards (this would be equivalent in power to existential types, which are not supported in Scala 3). Reduction in a covariant context would give
Any, which means that the recursive constructor disappears.

There was a bug in safeDealias that caused this behavior also for non-variant variables. This got fixed in the 2nd commit.

Fixes #15507

scala#15507 escaped the cycle detector since when the right hand side was typechecked,
the type constructor _NestedSet2's type was `[X] <: Any` and the constructor was marked Provisional.

Fixes scala#15507
odersky added 2 commits June 23, 2022 14:58
Providional type constructors are weird. They are still alias types but their
RHS is a possibly parameterized TypeBounds between Nothing and Any. safeDealias
should not "dealias" such a type to the upper bound `Any`.
... to avoid proliferation of flagsUNSAFE
@odersky odersky marked this pull request as ready for review June 23, 2022 13:23
@dwijnand dwijnand merged commit 327ecc2 into scala:main Jun 23, 2022
@dwijnand dwijnand deleted the fix-cyclic branch June 23, 2022 17:36
@odersky
Copy link
Contributor Author

odersky commented Jun 28, 2022

How I fixed it:

This one was puzzling. I tried to instrument the checkNonCyclic function to see what went wrong. I did that for the original definition

type _NestedSet1[X] = Set[_NestedSet1[?]]

which did not produce an error and the variant

type _NestedSet1[X] = Set[_NestedSet1[Int]]

which did produce an error. But at first I could not spot anything, mostly because I had to print the types in toString form since showing them with the i interpolator produced cycles.

It was when I did a diff between the instrumentations of the two definitions that I saw that NestedSet1 had completely disappeared from the computed right hand side of the type. Instead the RHS was computed as Set[Any].

That caused the investigation to switch to Namer where the RHS was computed. Here, to be able to bootstrap we first assume a type [X] >: Nothing <: Any for NestedSet1 and mark the symbol as Provisional. This is arguably very dirty, but looked like the only practicable way to deal with recursive definitions like this. Then when we computed the RHS we had a NestedSet1 <: ([X] => Nothing .. Any) for the constructor and the TypeBounds Nothing .. Any for the argument. This is an illegal application since it would be equivalent to an existential type \exists X.NestedSet1[X] and Scala 3 does not have existential types. The typer will try in TypeApplications to reduce such existential types away and report an error in PostTyper if that does not work. In this case the reduction did succeed, we end up with just Nothing..Any.

Aside: In fact because of another problem the reduction did not give the TypeBounds type Nothing..Any but just the upper bound. That was fixed in [030e867](https://github.com/lampepfl/dotty/pull/15508/commits/030e867fae1b67469d5c5ba5bdd77c97f4a30ffb).

In either case the whole type was now just Set[Any], or after the fix Set[? >: Nothing <: Any]. NestedSet1 has completely disappeared so no cycle was detected.

The fix was then comparatively simple. If we have a provisional type constructor, we do not allow wildcard arguments. Either the constructor will become an alias type, then it's a cyclic reference error, or it will become an abstract type, then it's an irreducible application to wildcard arguments. In either case it's wrong.

@Kordyjan Kordyjan added this to the 3.2.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cyclic reference in type is not detected
3 participants