https://github.com/dmaclach updated https://github.com/llvm/llvm-project/pull/212633
>From ffcdc4dff06d979caef40675422245d627a4c192 Mon Sep 17 00:00:00 2001 From: Dave MacLachlan <[email protected]> Date: Tue, 28 Jul 2026 14:58:16 -0700 Subject: [PATCH 1/3] [include-cleaner] Ensure receiver headers are kept when accessing ObjC properties When accessing Objective-C properties via dot-notation (e.g., obj.foo), include-cleaner was previously only recording the usage of the property itself or its underlying getter/setter methods. This could lead to cases where the header declaring the receiver's type (Interface or Protocol) was incorrectly flagged as unused if no other standard methods were invoked on it. --- .../include-cleaner/lib/WalkAST.cpp | 29 +++++++++++++ .../include-cleaner/unittests/WalkASTTest.cpp | 42 ++++++++++++++++--- 2 files changed, 66 insertions(+), 5 deletions(-) diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp index e3e610b8c33d8..c969038eceb8a 100644 --- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp +++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp @@ -458,6 +458,35 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { if (auto *Setter = E->getImplicitPropertySetter()) report(E->getLocation(), Setter); } + + // Report the receiver to ensure its declaring header is kept. + if (E->isObjectReceiver()) { + QualType Type = E->getBase()->IgnoreImpCasts()->getType(); + if (const auto *ObjCPtr = Type->getAs<ObjCObjectPointerType>()) { + if (auto *Interface = ObjCPtr->getInterfaceDecl()) { + report(E->getLocation(), Interface, RefType::Implicit); + } + for (const auto *Proto : ObjCPtr->quals()) { + report(E->getLocation(), const_cast<ObjCProtocolDecl *>(Proto), + RefType::Implicit); + } + } + } else if (E->isClassReceiver()) { + if (auto *Interface = E->getClassReceiver()) { + report(E->getLocation(), Interface, RefType::Implicit); + } + } else if (E->isSuperReceiver()) { + QualType Type = E->getSuperReceiverType(); + if (const auto *ObjCPtr = Type->getAs<ObjCObjectPointerType>()) { + if (auto *Interface = ObjCPtr->getInterfaceDecl()) { + report(E->getLocation(), Interface, RefType::Implicit); + } + for (const auto *Proto : ObjCPtr->quals()) { + report(E->getLocation(), const_cast<ObjCProtocolDecl *>(Proto), + RefType::Implicit); + } + } + } return true; } diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp index 6bf0bde9cfa10..42b4f04d44e39 100644 --- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp @@ -633,7 +633,7 @@ TEST(WalkAST, ObjCMessageExprClassReceiver) { TEST(WalkAST, ObjCPropertyRefExprExplicit) { testWalk(R"objc( - @interface MyClass + @interface $implicit^MyClass @property(nonatomic) int $explicit^foo; @end )objc", @@ -647,7 +647,7 @@ TEST(WalkAST, ObjCPropertyRefExprExplicit) { TEST(WalkAST, ObjCPropertyRefExprImplicitGetter) { testWalk(R"objc( - @interface MyClass + @interface $implicit^MyClass $explicit^- (int)foo; @end )objc", @@ -661,7 +661,7 @@ TEST(WalkAST, ObjCPropertyRefExprImplicitGetter) { TEST(WalkAST, ObjCPropertyRefExprImplicitSetter) { testWalk(R"objc( - @interface MyClass + @interface $implicit^MyClass $explicit^- (void)setFoo:(int)val; @end )objc", @@ -675,7 +675,7 @@ TEST(WalkAST, ObjCPropertyRefExprImplicitSetter) { TEST(WalkAST, ObjCPropertyRefExprExplicitSetter) { testWalk(R"objc( - @interface MyClass + @interface $implicit^MyClass @property(nonatomic) int $explicit^foo; @end )objc", @@ -689,7 +689,7 @@ TEST(WalkAST, ObjCPropertyRefExprExplicitSetter) { TEST(WalkAST, ObjCPropertyRefExprProtocol) { testWalk(R"objc( - @protocol MyProtocol + @protocol $implicit^MyProtocol @property(nonatomic) int $explicit^foo; @end )objc", @@ -701,6 +701,38 @@ TEST(WalkAST, ObjCPropertyRefExprProtocol) { {"-x", "objective-c"}); } +TEST(WalkAST, ObjCPropertyRefExprClassReceiver) { + testWalk(R"objc( + @interface $implicit^MyClass + @property(class, nonatomic) int $explicit^foo; + @end + )objc", + R"objc( + void test() { + int x = MyClass.^foo; + } + )objc", + {"-x", "objective-c"}); +} + +TEST(WalkAST, ObjCPropertyRefExprSuperReceiver) { + testWalk(R"objc( + @interface $implicit^ParentClass + @property(nonatomic) int $explicit^foo; + @end + @interface MyClass : ParentClass + @end + )objc", + R"objc( + @implementation MyClass + - (void)testSummary { + int x = super.^foo; + } + @end + )objc", + {"-x", "objective-c"}); +} + TEST(WalkAST, ObjCProtocolInType) { testWalk(R"objc( @protocol $explicit^MyProtocol >From 3db7c38260ab8d0766269b76fa6f4c8d6ccaa513 Mon Sep 17 00:00:00 2001 From: Dave MacLachlan <[email protected]> Date: Thu, 30 Jul 2026 13:23:04 -0700 Subject: [PATCH 2/3] - Added more tests to respond to comments - Fixed issues with tracking property invocations on `super` - Fixed issues with tracking of interfaces for selectors to match property support. --- .../include-cleaner/lib/WalkAST.cpp | 43 ++++-- .../include-cleaner/unittests/WalkASTTest.cpp | 128 +++++++++++++++++- 2 files changed, 160 insertions(+), 11 deletions(-) diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp index c969038eceb8a..d3b43295fceb0 100644 --- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp +++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp @@ -193,7 +193,7 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { } bool VisitCXXConstructExpr(CXXConstructExpr *E) { - // Always treat consturctor calls as implicit. We'll have an explicit + // Always treat constructor calls as implicit. We'll have an explicit // reference for the constructor calls that mention the type-name (through // TypeLocs). This reference only matters for cases where there's no // explicit syntax at all or there're only braces. @@ -416,11 +416,12 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { return true; } - bool VisitObjCMessageExpr(ObjCMessageExpr *E) { +bool VisitObjCMessageExpr(ObjCMessageExpr *E) { + auto startLoc = E->getSelectorStartLoc(); // Identify the selector and the method declaration if (auto *Method = E->getMethodDecl()) { // Report the method as a used symbol - report(E->getSelectorStartLoc(), Method); + report(startLoc, Method); } // If it's a class message, report the interface/class as used @@ -428,10 +429,25 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { if (auto *Interface = E->getReceiverInterface()) { report(E->getReceiverRange().getBegin(), Interface); } + } else { + if (auto *Interface = E->getReceiverInterface()) { + report(startLoc, Interface, RefType::Implicit); + } + QualType Type = E->getReceiverType(); + if (const auto *ObjCPtr = Type->getAs<ObjCObjectPointerType>()) { + for (auto *Proto : ObjCPtr->quals()) { + report(startLoc, Proto, RefType::Implicit); + } + } else if (const auto *ObjCType = Type->getAs<ObjCObjectType>()) { + for (auto *Proto : ObjCType->quals()) { + report(startLoc, Proto, RefType::Implicit); + } + } } return true; } + bool VisitObjCPropertyDecl(clang::ObjCPropertyDecl *PD) { reportType(PD->getLocation(), PD); return true; @@ -460,15 +476,14 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { } // Report the receiver to ensure its declaring header is kept. - if (E->isObjectReceiver()) { + if (E->isObjectReceiver()) { QualType Type = E->getBase()->IgnoreImpCasts()->getType(); if (const auto *ObjCPtr = Type->getAs<ObjCObjectPointerType>()) { if (auto *Interface = ObjCPtr->getInterfaceDecl()) { report(E->getLocation(), Interface, RefType::Implicit); } - for (const auto *Proto : ObjCPtr->quals()) { - report(E->getLocation(), const_cast<ObjCProtocolDecl *>(Proto), - RefType::Implicit); + for (auto *Proto : ObjCPtr->quals()) { + report(E->getLocation(), Proto, RefType::Implicit); } } } else if (E->isClassReceiver()) { @@ -481,9 +496,17 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { if (auto *Interface = ObjCPtr->getInterfaceDecl()) { report(E->getLocation(), Interface, RefType::Implicit); } - for (const auto *Proto : ObjCPtr->quals()) { - report(E->getLocation(), const_cast<ObjCProtocolDecl *>(Proto), - RefType::Implicit); + for (auto *Proto : ObjCPtr->quals()) { + report(E->getLocation(), Proto, RefType::Implicit); + } + } else if (const auto *ObjCType = Type->getAs<ObjCObjectType>()) { + // This is for handling `super.foo` property references. + if (auto *Interface = ObjCType->getInterface()) { + report(E->getLocation(), Interface, RefType::Implicit); + } + // The foo declaration could be in a protocol on super. + for (auto *Proto : ObjCType->quals()) { + report(E->getLocation(), Proto, RefType::Implicit); } } } diff --git a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp index 42b4f04d44e39..2a29fefc07a79 100644 --- a/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp +++ b/clang-tools-extra/include-cleaner/unittests/WalkASTTest.cpp @@ -605,7 +605,7 @@ TEST(WalkAST, ObjCImplementationDeclDependsOnInterface) { TEST(WalkAST, ObjCMessageExprSelectorLoc) { testWalk(R"objc( - @interface MyClass + @interface $implicit^MyClass $explicit^- (void)doSomething; @end )objc", @@ -617,6 +617,38 @@ TEST(WalkAST, ObjCMessageExprSelectorLoc) { {"-x", "objective-c"}); } +TEST(WalkAST, ObjCMessageExprSelectorLocProtocol) { + testWalk(R"objc( + @protocol $implicit^MyProtocol + $explicit^- (void)doSomething; + @end + )objc", + R"objc( + void test(id<MyProtocol> obj) { + [obj ^doSomething]; + } + )objc", + {"-x", "objective-c"}); +} + +TEST(WalkAST, ObjCMessageExprSelectorMessageChaining) { + testWalk(R"objc( + @interface $implicit^MyClass + $explicit^- (void)doSomething; + @end + @interface WrapperClass + - (MyClass *)myClass; + @end + )objc", + R"objc( + void test(WrapperClass *obj) { + // Weird space avoids Annotations thinking this is a range. + [ [obj myClass] ^doSomething]; + } + )objc", + {"-x", "objective-c"}); +} + TEST(WalkAST, ObjCMessageExprClassReceiver) { testWalk(R"objc( @interface $explicit^MyClass @@ -687,6 +719,62 @@ TEST(WalkAST, ObjCPropertyRefExprExplicitSetter) { {"-x", "objective-c"}); } +TEST(WalkAST, ObjCPropertyRefExprDesugaredSetter) { + testWalk(R"objc( + @interface $implicit^MyClass + @property(nonatomic) int $explicit^foo; + @end + )objc", + R"objc( + void test(MyClass *obj) { + [obj ^setFoo:42]; + } + )objc", + {"-x", "objective-c"}); +} + +TEST(WalkAST, ObjCPropertyRefExprDesugaredGetter) { + testWalk(R"objc( + @interface $implicit^MyClass + @property(nonatomic) int $explicit^foo; + @end + )objc", + R"objc( + void test(MyClass *obj) { + [obj ^foo]; + } + )objc", + {"-x", "objective-c"}); +} + +TEST(WalkAST, ObjCPropertyRefExprDesugaredClassSetter) { + testWalk(R"objc( + @interface MyClass + @property(class) int $explicit^foo; + @end + )objc", + R"objc( + void test() { + [MyClass ^setFoo:42]; + } + )objc", + {"-x", "objective-c"}); +} + +TEST(WalkAST, ObjCPropertyRefExprDesugaredClassGetter) { + testWalk(R"objc( + @interface MyClass + @property(class) int $explicit^foo; + @end + )objc", + R"objc( + void test() { + [MyClass ^foo]; + } + )objc", + {"-x", "objective-c"}); +} + TEST(WalkAST, ObjCPropertyRefExprProtocol) { testWalk(R"objc( @protocol $implicit^MyProtocol @@ -733,6 +821,44 @@ TEST(WalkAST, ObjCPropertyRefExprSuperReceiver) { {"-x", "objective-c"}); } +TEST(WalkAST, ObjCPropertyRefExprClassSuperReceiver) { + testWalk(R"objc( + @interface $implicit^ParentClass + @property(class, nonatomic) int $explicit^foo; + @end + @interface MyClass : ParentClass + @end + )objc", + R"objc( + @implementation MyClass + + (void)testSummary { + int x = super.^foo; + } + @end + )objc", + {"-x", "objective-c"}); +} + +TEST(WalkAST, ObjCPropertyRefExprClassSuperProtocolReceiver) { + testWalk(R"objc( + @protocol MyProtocol + @property(class) int $explicit^foo; + @end + @interface $implicit^ParentClass <MyProtocol> + @end + @interface MyClass : ParentClass + @end + )objc", + R"objc( + @implementation MyClass + + (void)testSummary { + int x = super.^foo; + } + @end + )objc", + {"-x", "objective-c"}); +} + TEST(WalkAST, ObjCProtocolInType) { testWalk(R"objc( @protocol $explicit^MyProtocol >From 0659049c1714d9938b640cd7832e5d569bcca6fc Mon Sep 17 00:00:00 2001 From: Dave MacLachlan <[email protected]> Date: Thu, 30 Jul 2026 13:28:18 -0700 Subject: [PATCH 3/3] Fixed up formatting --- clang-tools-extra/include-cleaner/lib/WalkAST.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp index d3b43295fceb0..12396758e5835 100644 --- a/clang-tools-extra/include-cleaner/lib/WalkAST.cpp +++ b/clang-tools-extra/include-cleaner/lib/WalkAST.cpp @@ -416,7 +416,7 @@ class ASTWalker : public RecursiveASTVisitor<ASTWalker> { return true; } -bool VisitObjCMessageExpr(ObjCMessageExpr *E) { + bool VisitObjCMessageExpr(ObjCMessageExpr *E) { auto startLoc = E->getSelectorStartLoc(); // Identify the selector and the method declaration if (auto *Method = E->getMethodDecl()) { @@ -447,7 +447,6 @@ bool VisitObjCMessageExpr(ObjCMessageExpr *E) { return true; } - bool VisitObjCPropertyDecl(clang::ObjCPropertyDecl *PD) { reportType(PD->getLocation(), PD); return true; @@ -476,7 +475,7 @@ bool VisitObjCMessageExpr(ObjCMessageExpr *E) { } // Report the receiver to ensure its declaring header is kept. - if (E->isObjectReceiver()) { + if (E->isObjectReceiver()) { QualType Type = E->getBase()->IgnoreImpCasts()->getType(); if (const auto *ObjCPtr = Type->getAs<ObjCObjectPointerType>()) { if (auto *Interface = ObjCPtr->getInterfaceDecl()) { _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
