Skip to content

Conversation

@b10902118
Copy link
Contributor

For #169824. The main purpose is to complete call hierarchy and reference links.

Local variables is marked at their init expression's beginning, which is intended for better understanding than marking at the end of the scope.

Current limitation is that it assumes a BindTemporaryExpr will always lead to a destructor invocation, which is not true when the temporary is elided, say at return statement.

…and delete

For llvm#169824

The main purpose is to complete call hierarchy and reference links.

Local variables is marked at their init expression's beginning, which is
intended for better understanding than marking at the end of the scope.

Current limitation is that it assumes a BindTemporaryExpr will always
lead to a destructor invocation, which is not true when the temporary is
elided, say at return statement.
@b10902118
Copy link
Contributor Author

#169824 (comment)

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.

Oh I excluded static variables because I was thinking of call graph, but their distructor will be invoked when the program terminates. I am not sure how to handle this.

@llvmbot
Copy link
Member

llvmbot commented Dec 11, 2025

@llvm/pr-subscribers-clang-tools-extra

@llvm/pr-subscribers-clangd

Author: Bill Tsui (b10902118)

Changes

For #169824. The main purpose is to complete call hierarchy and reference links.

Local variables is marked at their init expression's beginning, which is intended for better understanding than marking at the end of the scope.

Current limitation is that it assumes a BindTemporaryExpr will always lead to a destructor invocation, which is not true when the temporary is elided, say at return statement.


Full diff: https://github.com/llvm/llvm-project/pull/171860.diff

3 Files Affected:

  • (modified) clang-tools-extra/clangd/unittests/XRefsTests.cpp (+20-3)
  • (modified) clang/lib/Index/IndexBody.cpp (+17)
  • (modified) clang/lib/Index/IndexDecl.cpp (+11)
diff --git a/clang-tools-extra/clangd/unittests/XRefsTests.cpp b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
index 7ed08d7cce3d3..26cfeebbc1e02 100644
--- a/clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ b/clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -110,7 +110,7 @@ TEST(HighlightsTest, All) {
           [[~]]Foo() {};
         };
         void foo() {
-          Foo f;
+          Foo [[f]];
           f.[[^~]]Foo();
         }
       )cpp",
@@ -2242,15 +2242,32 @@ TEST(FindReferences, WithinAST) {
        class $def[[Foo]] {};
        void func($(func)[[Fo^o]]<int>);
       )cpp",
-      R"cpp(// Not touching any identifiers.
+      R"cpp(// Destructor not referenced by static identifiers.
         struct Foo {
           $def(Foo)[[~]]Foo() {};
         };
         void foo() {
-          Foo f;
+          static Foo f;
           f.$(foo)[[^~]]Foo();
         }
       )cpp",
+      R"cpp(// Destructor referenced by local varable.
+        struct Foo {
+          $def(Foo)[[^~]]Foo() {};
+        };
+        void foo() {
+          Foo $(foo)[[f]];
+        }
+      )cpp",
+      R"cpp(// Destructor referenced by temporary.
+        struct Foo {
+          $def(Foo)[[^~]]Foo() {};
+        };
+        void foo(Foo f) {
+          $(foo)[[Foo]]();
+          foo($(foo)[[f]]);
+        }
+      )cpp",
       R"cpp(// Lambda capture initializer
         void foo() {
           int $def(foo)[[w^aldo]] = 42;
diff --git a/clang/lib/Index/IndexBody.cpp b/clang/lib/Index/IndexBody.cpp
index 1979117d4695c..45fd1604816c4 100644
--- a/clang/lib/Index/IndexBody.cpp
+++ b/clang/lib/Index/IndexBody.cpp
@@ -161,12 +161,29 @@ class BodyIndexer : public RecursiveASTVisitor<BodyIndexer> {
   }
 
   bool VisitCXXDeleteExpr(CXXDeleteExpr *E) {
+    QualType DT = E->getDestroyedType();
+    if (!DT.isNull()) {
+      if (const auto *RecordTy = DT->getAsCXXRecordDecl()) {
+        if (const auto *Dtor = RecordTy->getDestructor()) {
+          IndexCtx.handleReference(Dtor, E->getExprLoc(), Parent, ParentDC);
+        }
+      }
+    }
+
     if (E->isGlobalDelete() || !E->getOperatorDelete())
       return true;
     return IndexCtx.handleReference(E->getOperatorDelete(), E->getBeginLoc(),
                                     Parent, ParentDC);
   }
 
+  bool VisitCXXBindTemporaryExpr(CXXBindTemporaryExpr *E) {
+    const CXXDestructorDecl *Dtor = E->getTemporary()->getDestructor();
+    if (Dtor) {
+      IndexCtx.handleReference(Dtor, E->getExprLoc(), Parent, ParentDC);
+    }
+    return true;
+  }
+
   bool VisitLabelStmt(LabelStmt *S) {
     if (IndexCtx.shouldIndexFunctionLocalSymbols())
       return IndexCtx.handleDecl(S->getDecl());
diff --git a/clang/lib/Index/IndexDecl.cpp b/clang/lib/Index/IndexDecl.cpp
index df875e0b40079..eac857586b374 100644
--- a/clang/lib/Index/IndexDecl.cpp
+++ b/clang/lib/Index/IndexDecl.cpp
@@ -307,6 +307,17 @@ class IndexingDeclVisitor : public ConstDeclVisitor<IndexingDeclVisitor, bool> {
     gatherTemplatePseudoOverrides(D, Relations);
     TRY_DECL(D, IndexCtx.handleDecl(D, SymbolRoleSet(), Relations));
     handleDeclarator(D);
+    // handle destructor reference for local variables
+    if (D->hasLocalStorage() && D->needsDestruction(D->getASTContext())) {
+      if (const auto *RecordTy = D->getType()->getAsCXXRecordDecl()) {
+        const auto *Dtor = RecordTy->getDestructor();
+        const Expr *InitExpr = D->getInit();
+        if (Dtor && InitExpr) {
+          IndexCtx.handleReference(Dtor, InitExpr->getExprLoc(), D,
+                                   D->getLexicalDeclContext());
+        }
+      }
+    }
     IndexCtx.indexBody(D->getInit(), D);
     return true;
   }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants