[NFC] More InOut Cleanup #12559
[NFC] More InOut Cleanup #12559
Conversation
|
@swift-ci please test |
lib/Sema/CSBindings.cpp
Outdated
| @@ -400,6 +400,11 @@ ConstraintSystem::getPotentialBindings(TypeVariableType *typeVar) { | |||
| if (type->hasError()) | |||
| continue; | |||
|
|
|||
| // If we're not allowed to bind to an InOut type, try to bind to the | |||
| // underlying type instead. | |||
| if (!typeVar->getImpl().canBindToInOut() && typeVar->getImpl().canBindToLValue()) | |||
There was a problem hiding this comment.
Oddly enough, it does the right thing. For this, the alternative is to let the binding through: Then we bind x to inout Int transitively and matchTypes never has a chance to suggest the fix.
func f0(_ : inout Int) {}
let g = { x in f0(x) }There was a problem hiding this comment.
Which is a symptom of inout inference being a thing at all with closures. It's not valid in general to propagate from the declref back to the parameter if there's an intervening inout. We should instead be trying to bind the parameter from the closure to the call's parameter but that's a larger change.
There was a problem hiding this comment.
Closures already use separate type variables for the 'external' parameter type and 'internal' parameter type. For example, if the 'external' type is inout T, the 'internal' type is @lvalue T. They are related with a BindParam constraint.
Maybe this is not plumbed all the way through.
In your example, getOpenedTypeOfReference() should return a type variable, say $T1, for x. The type of the closure expression as generated by CSGen should involve a different type variable, $T0. There should be a BindParam constraint between $T0 and $T1.
Am I thinking of something else?
There was a problem hiding this comment.
Closure inference specifically does not open the type of the declref for closure parameters and throws in its own BindParam constraint.
There was a problem hiding this comment.
That's just the part where we create a second 'internal' type variable and relate it with the original 'external' type variable via a BindParam constraint, which is what I was talking about above.
There was a problem hiding this comment.
so, can you figure out what's going wrong here? :-) I'm not convinced your change is the correct fix for the problem.
There was a problem hiding this comment.
Well that constraint is the root of all this trouble. I'll revert the last commit and come up with something more clever
There was a problem hiding this comment.
Could it be that 'edge contraction' or some other optimization pass is performing an invalid transformation on the constraint graph before the solver runs?
There was a problem hiding this comment.
I thought @xedin stopped that kind of contraction from being performed a while ago.
lib/Sema/TypeCheckConstraints.cpp
Outdated
| @@ -2180,6 +2180,27 @@ bool TypeChecker::typeCheckBinding(Pattern *&pattern, Expr *&initializer, | |||
| auto resultTy = typeCheckExpression(initializer, DC, contextualType, | |||
| contextualPurpose, flags, &listener); | |||
| if (resultTy) { | |||
| // If we detected that the initializer would | |||
|
@swift-ci please test source compatibility |
Move this from the validation phase into the phase where we check the binding. The type checker no longer has to reset the specifier for VarDecls.
|
Scaled back to trivial changes @swift-ci please smoke test and merge |
Trying to do two things here (and do them without hurting anything)
<InOutType>()- Teach CSBindings what to do with declrefs for closure parameters.