Index synthesized wrapper refs by substituting the Clang decl.

[dylansturg]

The goal of this change is replace the references to synthesized declarations, whose definitions are never contained in the indexstore, with references to the underlying declarations which are contained in the indexstore. The current behavior prevents code navigation from the references to the declaration.

Reference: #79912

• I added the EnumElementDecl generated from the underlying Clang EnumConstantDecl to the imported decl cache for ClangImporter.
• I took the effective Clang node and corresponding imported decl to substitute for "synthesized wrapper decls".

This has a weird outcome, where a reference to the Code enum inside of the synthesized struct can emit 2 references to the underlying enum. There's an example in the test case, where TestError.Code is referenced.

Fixes #79912

[bnbarham]

As I was looking at this I was wondering how/if cursor info handles this case. That then brought me to https://github.com/swiftlang/swift/blob/main/lib/AST/USRGeneration.cpp#L187 (added in 62f30d5).

We've apparently run into this before, where previously both the static variable and the enumerator had the same USR and that causes other issues (as then there's no way to uniquely identify the struct/enum and property/enumerator). I need to think about this a bit more - the USR returned from cursor info for these has to match the USR we add to the index.

[dylansturg]

I had also considered emitting index data for the synthesized declarations. It became complex because the Swift indexing components don't emit ideal data from the Module with the Clang decls. For example, file references are disabled when indexing a module without Swift source files.

I think it could make sense to emit the synthesized declarations. I previously enabled that for decls synthesized from Swift (e.g. the synthesized Codable conformance).

[bnbarham]

I think my preference here would just be to move that USR adjustment logic into initDocEntityInfo only. Then everything else should just work the way you'd expect?

[dylansturg]

I think my preference here would just be to move that USR adjustment logic into initDocEntityInfo only.

Is the intent for the USR for the synthesized struct type and its static accessors use the USRs of the Clang enum and enum constants (respectively), for the index data and cursor info? And in the doc info output, the struct and accessors have unique USRs that are different from the underlying Clang enum and enum constants?

That seems feasible but I'm not sure how to move that USR adjustment. After I remove the adjustment logic from USRGenerationRequest, then requests will use the underlying Clang node to generate a USR for the static accessors. Which is reasonable when generating index data. When initDocEntityInfo needs the Swift USR for the static accessors and wants to ignore the Clang node, does it need to generate the USR in a different way because USRGenerationRequest is going to use the underlying Clang node?

[bnbarham]

Is the intent for the USR for the synthesized struct type and its static accessors use the USRs of the Clang enum and enum constants (respectively), for the index data and cursor info?

Yeah, for anything that doesn't specifically need the synthesized Swift declarations, which is just documentation I believe (so doc info + symbol graph).

That seems feasible but I'm not sure how to move that USR adjustment

I don't have much better of an idea here other than passing a flag through (and then we could keep it there). Pretty ugly though :( Any other ideas @hamishknight?

[dylansturg]

Yeah, for anything that doesn't specifically need the synthesized Swift declarations, which is just documentation I believe (so doc info + symbol graph).

That sounds reasonable to me. I added a flag and, I think, got it working in those places. I noticed that symbol graph wasn't specifically testing for the USR of the synthesized declarations, so I updated the relevant test to check that.

I wrote the USRGenerationOptions to try have a single point of documentation for the emit_synthesized_decls flag plus allow for some flexibility in the future. But using it in the various print*USR APIs proved fairly painful so I just kept the simple bool. I'm open to other approaches there.

@bnbarham Can you take another look and let me know what you think? Thanks!

[hamishknight]

I don't have much better of an idea here other than passing a flag through (and then we could keep it there). Pretty ugly though :( Any other ideas @hamishknight?

Yeah, I'm okay with passing a flag through

[dylansturg]

I think I have everything updated. Please take another look. Thanks!

[hamishknight]

One last comment, but otherwise this LGTM, thanks!

[QuietMisdreavus]

Thanks for updating this test! This would have been my only concern with this PR, but it looks like that's settled.

[bnbarham]

One tiny fix-up, otherwise LGTM. Thanks @dylansturg!

[hamishknight]

@swift-ci please test

[drexin]

@swift-ci test