Skip to content

Fix #6247: Heal selection errors by fully defining qualifier type #6257

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

Closed
wants to merge 1 commit into from

Conversation

odersky
Copy link
Contributor

@odersky odersky commented Apr 7, 2019

This solves a situation where we cannot find a member m of a qualifier of type T
where T is a type variable with lower bound L, where m is a member of L.
This has come up occasionally before, but never until now in minimized form. The
solution is to instantiate the type variable using fullyDefinedType and try again.

This solves a situation where we cannot find a member `m` of a qualifier of type `T`
where `T` is a type variable with lower bound `L`, where `m` is a member of `L`.
This has come up occasionally before, but never until now in minimized form. The
solution is to instantiate the type variable using `fullyDefinedType` and try again.
@odersky odersky requested a review from smarter April 7, 2019 16:52
val x1: String|Null = null
x1.nn.length
val x2: String = x1.nn
x1.nn.length
Copy link
Member

Choose a reason for hiding this comment

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

This line failed while the identical expression two lines above succeeded, so something fishy is going on, I've got what I think is a proper fix in #6301.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Great! But I think it would be still an improvement to have the changes in this PR also. It is insurance against the fact that we are more lazy instantiating type variables than scalac. Do you agree?

Copy link
Member

Choose a reason for hiding this comment

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

Without an example showing that this is needed I'm not sure, there's now been two actual bugs we caught that would have stayed hidden with this PR in: #6301 and 38c6d46. so I'd rather keep things as is for now.

Copy link
Contributor Author

@odersky odersky Apr 15, 2019

Choose a reason for hiding this comment

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

I agree. I just went through all the inference related // dotty deviation comments in the compiler codebase and could remove them all without this fix. Except one which was related to us dropping weak conformance. So let's leave things as they are.

@odersky odersky closed this Apr 15, 2019
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.

2 participants