From 32946ddd2e5ab83505e832b7ef43bc10bd6dce68 Mon Sep 17 00:00:00 2001
From: Daan De Meyer <daan.j.demeyer@gmail.com>
Date: Wed, 21 May 2025 16:40:30 +0200
Subject: [PATCH] [clang-include-cleaner] Make cleanup attr report expr
 location (#140233)

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 ba6eff49e9c9..baff90faa6ea 100644
--- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
+++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp
@@ -322,7 +322,7 @@ public:
   }
 
   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 19695a34bd63..0de0b77f33da 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 a6a7482a94a2..06462b8a26bc 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 4d7f0455444f..ac77a7c2c58a 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,