Fix #9568: Disallow partially undefined types as byname anchors #9575
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
smarter
reviewed
Aug 17, 2020
| @@ -1371,8 +1376,12 @@ trait Implicits: | |||
| history match | |||
| case prev @ OpenSearch(cand1, tp, outer) => | |||
| if cand1.ref eq cand.ref then | |||
| def checkFullyDefined = | |||
| val result = isFullyDefined(tp, ForceDegree.failBottom) | |||
There was a problem hiding this comment.
What would happen if we forced the type to be fully defined at that point instead of disqualifying it?
There was a problem hiding this comment.
I guess it will only be disqualified if a type variable would be instantied to Nothing, but that might be a legitimate instantiation
There was a problem hiding this comment.
Yes, I don't know what is the right force degree, or whether there even is one.
There was a problem hiding this comment.
I've tried setting the force degree to all which changes how we do error reporting but seems to be fine, let me know what you think.
There was a problem hiding this comment.
I think that's fine, too. It's a corner case, so the simpler the handling the better.
Require that the type that a by-name implicit can recursively implement must be fully defined.
smarter
force-pushed
the
fix-#9568
branch
2 times, most recently
from
36e0755 to
e8b94f0
Compare
Nothing has legitimate use as a type parameter, so special-casing inference to reject it can cause issues. Also by removing this special-case we can remove the error reporting logic dedicated to dealing with non-fully-defined types, instead the user will see that a type parameter was instantiated to `Nothing` in the error which should be clear enough.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Require that the type that a by-name implicit can recursively implement
must be fully defined.