[clangd] Implicit destructors not recorded as reference

[b10902118]

Currently clangd’s call hierarchy and reference don’t include destructors, making the analysis incomplete, for example: #157876

While there is no actual position for destructor, I think it is reasonable to be at the same location of the corresponding constructor because it is decided there. I am new to clang AST but I think the information can be infered from it. Just found #166110. Currently I am looking into clang-tools-extra/clangd/index/SymbolCollector.cpp, not sure whether in the right direction.

But I am also unsure whether adding destructor will break existing rules. Need help.

[llvmbot]

@llvm/issue-subscribers-clangd

Author: Bill Tsui (b10902118)

Currently clangd’s call hierarchy and reference don’t include destructors, making the analysis incomplete, for example: https://github.com//issues/157876

While there is no actual position for destructor, I think it is reasonable to be at the same location of the corresponding constructor because it is decided there. I am new to clang AST but I think the information can be infered from it. Just found #166110. Currently I am looking into clang-tools-extra/clangd/index/SymbolCollector.cpp, not sure whether in the right direction.

But I am also unsure whether adding destructor will break existing rules. Need help.

[HighCommander4]

While there is no actual position for destructor, I think it is reasonable to be at the same location of the corresponding constructor because it is decided there.

That sounds reasonable to me for variables with automatic or static storage duration.

(For dynamic allocations, the destructor reference location should be at the delete expression.)

Currently I am looking into clang-tools-extra/clangd/index/SymbolCollector.cpp, not sure whether in the right direction.

SymbolCollector is one of the places that would need changes, yeah. It is used for finding references across translation units.

There is also a codepath for finding references within the current translation unit, which is around here.

As it happens, there is currently some ongoing work on another find-references enhancement in clangd/clangd#716. You might find some of the starting points I gave for that in this comment helpful for your change as well.

But I am also unsure whether adding destructor will break existing rules.

Can you elaborate on what sort of rules you have in mind?

Need help.

I'm happy to try to answer other questions you might run into.

[HighCommander4]

See also a couple of existing clangd issues which are somewhat related:

• constructor call not found for member object clangd/clangd#1312 (about recording references to implicitly called constructors)
• Finding references to destructor finds too many or too few. clangd/clangd#1321 (about recording references to destructors in delete expressions)