Harden Type Inference #12560
Merged
Harden Type Inference #12560
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
|
Based on #12535. Best reviewed commit by commit. |
The purpose of canDefineFurther is to discover new members. That is not helped by instantiating a type variable to `Nothing`. Avoiding the instantiation helpos error messages. There was one test (i864.scala) that fails now but compiled previously with a surprising instantiation to Nothing. I believe it is better to issue an error message in this case.
We should retry adapt if we can instantiate typevars in either the type of the tree or the expected type
This was deemed too hard before for type inference, but it works now.
smarter
approved these changes
May 25, 2021
Comment on lines
+347
to
+348
| MonadFlatten.flattened(List(List(1, 2, 3), List(4, 5))) // ok, synthesizes (using ListMonad) | ||
| MonadFlatten.flattened(List(List(1, 2, 3), List(4, 5)))(using ListMonad) // error |
There was a problem hiding this comment.
To be clear: passing ListMonad explicitly already failed on master and this PR doesn't improve the situation but just documents it?
Comment on lines
1135
to
1137
| case SAMType(sam @ MethodTpe(_, formals, restpe)) => | ||
| case RefinedType(parent, nme.apply, mt @ MethodTpe(_, formals, restpe)) | ||
| if defn.isNonRefinedFunction(parent) && formals.length == defaultArity => | ||
| (formals, untpd.DependentTypeTree(syms => restpe.substParams(mt, syms.map(_.termRef)))) |
There was a problem hiding this comment.
Do we have a testcase for this change?
There was a problem hiding this comment.
In fact I could not come up with one. The only test that fails in the SAM case was scala/reflect/TypeTest.scala. Making the analogous example based on dependent functions instead of SAMs passed even without this patch. I am going to include that test. I have reverted the change, since it's not clear we need it.
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.
Various improvements to make type inference and internal checking more robust. Best reviewed commit by commit.
These all address issues that were first uncovered when trying to run the type inferencer on inferred code again, where all type variables are reset to undefined. So the program is different since it contains adaptations like implicit arguments, but the
type variables are inferred again.