swift-index-store
swift-index-store copied to clipboard
Certain extensions USRs aren't demangled properly
#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.
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 🙃
Your own protocolext repro on https://github.com/apple/swift/issues/64612 has examples in Module1.swift (s:e:s:7Module03FooVs:7Module03BarP)
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