Certain extensions USRs aren't demangled properly

[pennig]

#14 improved demangling by trying to handle extension USRs (which have an s:e: prefix). More work is required though. In some cases, the USR for an extension can be of the form s:e:s:<type USR>s:<protocol USR>, whereby multiple USRs are joined with the same s:e: prefix. I've noticed this in one of my own index stores, and confirmed this behavior here: https://github.com/apple/swift/blob/3e3a98f80191821a63e76a7d7cc4b1371ff3e493/lib/AST/USRGeneration.cpp#L343

Not sure why this is done instead of writing a USR which leverages the Extension or ExtensionDescriptor node to create a taller tree around the extension, but regardless of how the USR is generated, I don't think libswiftDemangler handles this construction.

Seems like the path forward is to check for the s:e: prefix, then scan for each s:, and demangle each. But this would also require updating func demangle(symbol: String) to return [DemangledNode]?, or inventing a new kind of DemangledNode that can be the parent of each extension USR. Could also just use Extension and let it ride.

[keith]

Interesting. Do you have an example one I can work with for this? (I might be able to find one in ours). I do worry that at some point we just shouldn't be trying to get 100% accurate as it's definitely documented that USRs should be treated as opaque identifiers, it just so happens to work like this 🙃

[pennig]

Your own protocolext repro on apple/swift#64612 has examples in Module1.swift (s:e:s:7Module03FooVs:7Module03BarP)

[keith]

oh yea thanks, that's an interesting one. it's also interesting because there are more useful usrs around that same place:

/*
          struct=Foo=s:7Module03FooV=reference, extendedBy
          extension.swiftExtensionOfStruct=Foo=s:e:s:7Module03FooVs:7Module03BarP=definition
          |    protocol=Bar=s:7Module03BarP=reference, baseOf
          v    v  */
extension Foo: Bar {}

which potentially covers some cases where we want to see what we're referencing. but i agree that we should add a new API that handles this if we want to. I think we should explicitly make it USR specific, and maybe even move the current handling there to make it less ambiguous