Skip to content

Commit 32946dd

Browse files
authored
[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
1 parent 95bd9ee commit 32946dd

File tree

4 files changed

+17
-4
lines changed

4 files changed

+17
-4
lines changed

clang-tools-extra/include-cleaner/lib/WalkAST.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -322,7 +322,7 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> {
322322
}
323323

324324
bool VisitCleanupAttr(CleanupAttr *attr) {
325-
report(attr->getLocation(), attr->getFunctionDecl());
325+
report(attr->getArgLoc(), attr->getFunctionDecl());
326326
return true;
327327
}
328328

clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -573,7 +573,7 @@ TEST(WalkAST, OperatorNewDelete) {
573573

574574
TEST(WalkAST, CleanupAttr) {
575575
testWalk("void* $explicit^freep(void *p);",
576-
"void foo() { __attribute__((^__cleanup__(freep))) char* x = 0; }");
576+
"void foo() { __attribute__((__cleanup__(^freep))) char* x = 0; }");
577577
}
578578

579579
} // namespace

clang/include/clang/Basic/Attr.td

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1354,6 +1354,17 @@ def Cleanup : InheritableAttr {
13541354
let Args = [DeclArgument<Function, "FunctionDecl">];
13551355
let Subjects = SubjectList<[LocalVar]>;
13561356
let Documentation = [CleanupDocs];
1357+
// FIXME: DeclArgument should be reworked to also store the
1358+
// Expr instead of adding attr specific hacks like the following.
1359+
// See the discussion in https://github.com/llvm/llvm-project/pull/14023.
1360+
let AdditionalMembers = [{
1361+
private:
1362+
SourceLocation ArgLoc;
1363+
1364+
public:
1365+
void setArgLoc(const SourceLocation &Loc) { ArgLoc = Loc; }
1366+
auto getArgLoc() const { return ArgLoc; }
1367+
}];
13571368
}
13581369

13591370
def CmseNSEntry : InheritableAttr, TargetSpecificAttr<TargetARM> {
@@ -4815,7 +4826,7 @@ def HLSLResourceBinding: InheritableAttr {
48154826
void setImplicitBindingOrderID(uint32_t Value) {
48164827
ImplicitBindingOrderID = Value;
48174828
}
4818-
bool hasImplicitBindingOrderID() const {
4829+
bool hasImplicitBindingOrderID() const {
48194830
return ImplicitBindingOrderID.has_value();
48204831
}
48214832
uint32_t getImplicitBindingOrderID() const {

clang/lib/Sema/SemaDeclAttr.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3620,7 +3620,9 @@ static void handleCleanupAttr(Sema &S, Decl *D, const ParsedAttr &AL) {
36203620
return;
36213621
}
36223622

3623-
D->addAttr(::new (S.Context) CleanupAttr(S.Context, AL, FD));
3623+
auto *attr = ::new (S.Context) CleanupAttr(S.Context, AL, FD);
3624+
attr->setArgLoc(E->getExprLoc());
3625+
D->addAttr(attr);
36243626
}
36253627

36263628
static void handleEnumExtensibilityAttr(Sema &S, Decl *D,

0 commit comments

Comments
 (0)