Skip to content
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

Always use adapted type in withDenotation #16901

Merged
merged 5 commits into from
Feb 16, 2023
Merged

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Feb 13, 2023

When creating a NamedType with a given overloaded denotation, make sure that the type has a Name as designator. This prevents accidentally overwriting a more precise symbolic TermRef that refers to one specific alternative of the denotation.

This might be enough to fix #16884.

EDIT: It wasn't enough but the second commit 46e82dd should fix it. The second commit never overwrites in withDenot. It can do that because we fix infoDependsOnPrefix to work correctly for abstract types that are refined in a self type. It turned out that previously we needed some TypeRefs to keep their Name designators because that way we would recompute their info with a member operation. If these TypeRefs had a symbol designator they would be recomputed wrongly by symd.current in fromDesignator because the preceding infoDependsOnPrefix test was faulty.

It would be great if we could maintain the general invariant that NamedTypes with overloaded denotations always have names as designators. But that looks very hard when we take into account that we need to update named types to new runs. A type might have a single denotation in one round and an overloaded one in the next.

When creating a NamedType with a given overloaded denotation, make sure that
the type has a Name as designator. This prevents accidentally overwriting a more
precise symbolic TermRef that refers to one specific alternative of the denotation.

This might be enough to fix scala#16884.

It would be great if we could maintain the general invariant that NamedTypes with
overloaded denotations always have names as designators. But that looks very hard
when we take into account that we need to update named types to new runs.
A type might have a single denotation in one round and an overloaded one in the
next.
Make infoDependsOnPrefix return true even if the prefix is
the ThisType of the enclosing class, in case that class has a selftype
which refines the binding of an abstract type.

This allows to implement withDenot without overwriting a NamedType's state.
@odersky odersky changed the title Tweak withDenot for overloaded denotations Always use adapted type in withDenotation Feb 13, 2023
Also consider non-final term members with self type refinements for infoDependsOnPrefix
@odersky
Copy link
Contributor Author

odersky commented Feb 14, 2023

The hard part of this PR was to come up with a definition of infoDependsOnPrefix that is precise and does not cause infinite recursions when called from NamedType.denot.

But correctly diagnosing the problem in #16884 looks even harder! So, props to @smarter for that.

Copy link
Member

@smarter smarter left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that this fixes #16884 in practice.

@smarter smarter assigned odersky and unassigned smarter Feb 16, 2023
@smarter smarter merged commit 2507577 into scala:main Feb 16, 2023
@smarter smarter deleted the fix-16884 branch February 16, 2023 21:52
@Kordyjan Kordyjan added this to the 3.3.1 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Nondeterministic compiler crash on clean compile (ClassCastException: PolyType, CachedTermRef)
3 participants