[clang] [clang-tools-extra] [clang-include-cleaner] Make cleanup attr report expr location (PR #140233)

[Daan De Meyer via cfe-commits]

https://github.com/DaanDeMeyer updated 
https://github.com/llvm/llvm-project/pull/140233

>From 57687d8eebc1f8e2b59ed131791895d76f88e30a Mon Sep 17 00:00:00 2001
From: Daan De Meyer <daan.j.deme...@gmail.com>
Date: Fri, 16 May 2025 12:08:47 +0200
Subject: [PATCH] [clang-include-cleaner] Make cleanup attr report expr
 location

Instead of reporting the location of the attribute, let's report
the location of the function reference that's passed to the cleanup
attribute as the first argument. This is required as the attribute
might be coming from a macro which means clang-include-cleaner skips
the use as it gets attributed to the header file declaringt the macro
and not to the main file.

To make this work, we have to add a fake argument to the CleanupAttr
constructor so we can pass in the original Expr alongside the function
declaration.

Fixes #140212
---
 clang-tools-extra/include-cleaner/lib/WalkAST.cpp   |  2 +-
 .../include-cleaner/unittests/WalkASTTest.cpp       |  2 +-
 clang/include/clang/Basic/Attr.td                   | 13 ++++++++++++-
 clang/lib/Sema/SemaDeclAttr.cpp                     |  4 +++-
 4 files changed, 17 insertions(+), 4 deletions(-)

diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp 
b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
index ba6eff49e9c98..baff90faa6eae 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -322,7 +322,7 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
   }
 
   bool VisitCleanupAttr(CleanupAttr *attr) {
-    report(attr->getLocation(), attr->getFunctionDecl());
+    report(attr->getArgLoc(), attr->getFunctionDecl());
     return true;
   }
 
diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp 
b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
index 19695a34bd63e..0de0b77f33daf 100644
--- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
+++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp
@@ -573,7 +573,7 @@ TEST(WalkAST, OperatorNewDelete) {
 
 TEST(WalkAST, CleanupAttr) {
   testWalk("void* $explicit^freep(void *p);",
-           "void foo() { __attribute__((^__cleanup__(freep))) char* x = 0; }");
+           "void foo() { __attribute__((__cleanup__(^freep))) char* x = 0; }");
 }
 
 } // namespace
diff --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index a6a7482a94a29..06462b8a26bc0 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -1354,6 +1354,17 @@ def Cleanup : InheritableAttr {
   let Args = [DeclArgument<Function, "FunctionDecl">];
   let Subjects = SubjectList<[LocalVar]>;
   let Documentation = [CleanupDocs];
+  // FIXME: DeclArgument should be reworked to also store the
+  // Expr instead of adding attr specific hacks like the following.
+  // See the discussion in https://github.com/llvm/llvm-project/pull/14023.
+  let AdditionalMembers = [{
+private:
+    SourceLocation ArgLoc;
+
+public:
+    void setArgLoc(const SourceLocation &Loc) { ArgLoc = Loc; }
+    auto getArgLoc() const { return ArgLoc; }
+  }];
 }
 
 def CmseNSEntry : InheritableAttr, TargetSpecificAttr<TargetARM> {
@@ -4815,7 +4826,7 @@ def HLSLResourceBinding: InheritableAttr {
       void setImplicitBindingOrderID(uint32_t Value) {
         ImplicitBindingOrderID = Value;
       }
-      bool hasImplicitBindingOrderID() const { 
+      bool hasImplicitBindingOrderID() const {
         return ImplicitBindingOrderID.has_value();
       }
       uint32_t getImplicitBindingOrderID() const {
diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 4d7f0455444f1..ac77a7c2c58a5 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -3620,7 +3620,9 @@ static void handleCleanupAttr(Sema &S, Decl *D, const 
ParsedAttr &AL) {
     return;
   }
 
-  D->addAttr(::new (S.Context) CleanupAttr(S.Context, AL, FD));
+  auto *attr = ::new (S.Context) CleanupAttr(S.Context, AL, FD);
+  attr->setArgLoc(E->getExprLoc());
+  D->addAttr(attr);
 }
 
 static void handleEnumExtensibilityAttr(Sema &S, Decl *D,

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits