[CSBindings] Avoid binding type variables to collection types directly

[xedin]

Instead of binding collection types directly let's try to
bind using temporary type variables substituted for element
types, that's going to ensure that subtype relationship is
always preserved.

Resolves: rdar://problem/35541153

[xedin]

@DougGregor This is the idea I had for fixing array subtyping problem when passed as an argument. WDYT?

[slavapestov]

@xedin It seems that either the fix should be more fundamental, or any explicit enumeration of these cases should also include optionals and any other covariant type constructor (functions? tuples?). What's the actual underlying problem here?

[xedin]

@slavapestov Here is the problem this is trying to solve:

func foo<U: Equatable, V: Equatable, C: Collection>(_ c: C) -> String where C.Element == (U, V) {
  return "OK"
}

let x: [(a: Int, b: Int)] = []
foo(x)

Currently this is not going to compile because deep equality restriction results in tuples being compared as equal where instead argument should be upcast and then compared. So what my code (just a simple experiment) is trying to do is instead of use temporary type variable to bind to element type and match collection through that e.g.

[(a: Int, b: Int)] arg conv $T
$T.Element equal ($T1, $T2)

is what is going to be generated when solver matches x to parameter type, so getPotentialBindings simply picks it up and binds T to [(a: Int, b: Int)] directly which results in
$T.Element equal ($T1, $T2) failure because labels do not much. But if instead we do the following: generate new constraint $T bind Array<$T3> and leave previous constraints intact $T3 is going to facilitate generation of array-upcast constraint which is going to erase the labels and successfully match element type.

I think better solution would be to move this logic into getPotentialBindings and only do it is binding comes from subtype and/or conversion constraint but I just didn't want to spend to much time on this if @DougGregor thinks that this is not the right approach.

[DougGregor]

It's probably worth calling reconstituteSugar, as in the previous code, so we get prettier types.

[DougGregor]

This is effectively dropping all information about the type arguments to Array / Set / Dictionary. I guess that's okay because whatever constraint we inferred this type binding from will still be there to help bind these type variables.

[xedin]

Will do re: reconstituteSugar, was wondering if we really need it there... Regarding information - yes, that's the intent, to use fresh type variables here so conversion could be done properly.

[DougGregor]

FWIW, in cases where getAs should never fail, you should use castTo to always get a non-null pointer (or assert if it's not of the right type).

[xedin]

Thanks, I didn't know that!

[DougGregor]

This should probably be an else if clause of the if above, since we don't want to look back at a type that we just opened to type variables.

[xedin]

@DougGregor I've made it so potential bindings now track their source kind and when we attempt the bindings we only do open again for collections if they type comes from argument conversion. WDYT?

[xedin]

@swift-ci please test

[xedin]

@swift-ci please test source compatibility

[DougGregor]

"else if"?

[xedin]

Yes, I forgot to change that from previous version, will do!

[DougGregor]

Could you add a second example here where the the function in question takes two parameters? I want to be sure that ParenType and TupleType function inputs don't end up getting different behavior.

[xedin]

Sure!

[xedin]

@DougGregor All done!

[xedin]

@swift-ci please test

[xedin]

@swift-ci please test source compatibility