[ASTPrinter] Fix issue printing Self resolved to a generic type

[ahoppen]

In some situations (see added test case), we end up with types that contain both an archetype and a generic type parameter. Trying to map them out of context would fail because mapTypeOutOfContext asserts that there are no type parameters in the type.

IIUC leaving the generic type parameters unchanged in MapTypeOutOfContext and removing the assertion should be the intended behaviour.

Fixes rdar://76021569

[slavapestov]

I think this will mask other bugs. Types should never contain a mix of archetypes and interface types. Can we fix the downstream issue in the IDE code instead?

[ahoppen]

Thanks, for that info @slavapestov. I wasn’t sure if these mixed types really should never occur or if they just happened to not occur so far.

I have updated the PR to compute the interface type first before substituting it in for Self. Could you take another look?

[slavapestov]

Thanks, this is more maintainable, in my opinion at least!

[ahoppen]

Thanks, this is more maintainable, in my opinion at least!

I agree.

@akyrtzi This change caused some USRs to change. Could you take a look if this is can be considered a fix, problem or something we don’t have to care about?

In this definition

extension S3: P6 where Wrapped: P6 {}

The USR of the Wrapped generic param changed from

- s:4cake2S3VA2A2P6RzrlE7Wrappedxmfp ---> (extension in cake):cake.S3< where A: cake.P6>.Wrapped
+ s4cake2S3V7Wrappedxmfp ---> cake.S3.Wrapped

And here

extension D: _SomeProto where T: Equatable {
    public typealias Item = T
    
}

The USR of T changed as follows:

- s:16UnderscoredProto1DVAASQRzrlE1Txmfp ---> (extension in UnderscoredProto):UnderscoredProto.D< where A: Swift.Equatable>.T
+ s:16UnderscoredProto1DV1Txmfp ---> UnderscoredProto.D.T

[ahoppen]

After quite a bit of investigation, I think there is a bigger underlying issue with the USRs here that’s out of the scope of this PR to fix.

Each extension that makes restrictions on generic parameters redeclares those parameters with implicit GenericParamDecls. These new implicit declarations also encode the restrictions. Currently, in extensions we use these redeclared parameters. I think this is already counter-intuitive in terms of USRs because e.g. renaming a generic parameter in the original definition should also rename it in any extensions.

With this change, things start falling apart once we introduce typealiass. In a simplified example consider:

class MyClass<T> {
  typealias MyAlias = T
}
extension MyClass where T: Equatable {
  var x: MyAlias
}

There are two ways we can interpret MyAlias, which are AFAICT equivalent for all purposes apart from USRs:
1.) MyAlias refers to T declared in the extension, thus behaving more like a preprocessor statement (this is the current behavior)
2.) MyAlias refers to T declared in MyClass because we do a lookup on MyAlias, leading us into the definition of MyClass in which we know nothing of the extension.

My proposal would be to

• Merge this PR as-is to resolve the crash in rdar://76021569, accepting that the USR assignment for generic params is somewhat broken and was so even before this PR

and in a follow-up PR

• Canonicalize all generic params to refer to their original definition (opposed to their re-definition) in ASTPrinter. That way global rename would be able to rename also the generic params in extensions.

@akyrtzi What do you think?

[ahoppen]

@swift-ci Please smoke test

[akyrtzi]

SGTM! Thanks for digging into this and discovering the issue!

[ahoppen]

@swift-ci Please test

[swift-ci]

Build failed
Swift Test OS X Platform
Git Sha - 9ba892c

[ahoppen]

@swift-ci Please test macOS