AST: Stop pretending that conformance lookup takes the module into account

[slavapestov]

It has never taken the module into account, and callers are just passing in the nearest random module they can reach from whatever other AST object they have on hand.

[slavapestov]

@swift-ci Please smoke test

[rjmccall]

I guess the right question is: should it take the module into account?

If we're ignoring the module, what actually happens if we can see conformances in multiple modules? Do we just resolve to one arbitrarily?

[slavapestov]

If we're ignoring the module, what actually happens if we can see conformances in multiple modules? Do we just resolve to one arbitrarily?

We return the first conformance output by NominalTypeDecl::lookupConformance(), in whatever order they're stored in within the ConformanceLookupTable.

[slavapestov]

@swift-ci Please smoke test

[jckarter]

I guess the right question is: should it take the module into account?

I think you're both right: it should take the module into account, but in practice it doesn't, and few uses of lookupConformance make the effort to pass a principled value, which is no better than not having the parameter at all, as far as what it would take to retrofit doing it right would be. Also, in most cases, the proper fix for a lookupConformance call is that the lookup shouldn't be necessary in the first place since the conformance ought to have been carried forward through some abstraction that currently doesn't. So I think I'm ok with this.

[slavapestov]

I think the disambiguating rule needs to be global anyway, for example if one conformance is retroactive and the other isn’t, we should prefer the non-retroactive one. If both are retroactive we should return an invalid conformance (possibly encoding the overlap for downstream diagnostics).

[rjmccall]

If we're ignoring the module, what actually happens if we can see conformances in multiple modules? Do we just resolve to one arbitrarily?

We return the first conformance output by NominalTypeDecl::lookupConformance(), in whatever order they're stored in within the ConformanceLookupTable.

Okay. My language design sense is that, to the extent that we want to support multiple conformances, we probably need some level of site-specific disambiguation analogous to C++'s using directives (which can appear in local scopes) and scope qualifiers (which can appear in a specific member reference). That means disambiguating by module is not the right API, either, so there's no good reason to insist on keeping it if it's not currently accomplishing anything. In a world with local disambiguation like that, there would still have to be a parameter to the lookup, but it's a parameter we have no hope of faking in random places in the compiler, so we would need changes to all of these lookups anyway. Specific use cases like dynamic cast optimization should use a specific API that emulates whatever the dynamic semantics are intended to be, with the potential to fail and report that it cannot make the decision statically. Most of the rest of these lookups should simply cease to exist, replaced by being smarter about propagating conformance references down from the type-checker.

If we're in agreement that that's the right future direction, then yeah, I think this should move forward.

[slavapestov]

swiftlang/llvm-project#8950
@swift-ci Please smoke test

[beccadax]

Based on your discussion with John and Joe, this looks like a reasonable change.

My only question is, why not go further? Now that these methods are static, putting them in ModuleDecl seems like a pretty arbitrary choice. Perhaps they should be on Type instead, and the *InModule* helper types should become *InType* to match?

[slavapestov]

My only question is, why not go further? Now that these methods are static, putting them in ModuleDecl seems like a pretty arbitrary choice. Perhaps they should be on Type instead, and the InModule helper types should become InType to match?

Well, my original plan was to convert call sites incrementally, because you can still write fooModule->lookupConformance() to call a static method in C++, but I ended up doing most of them anyway.

I'll do the renaming in a separate PR.

[slavapestov]

swiftlang/llvm-project#8950
@swift-ci Please smoke test