Skip to content

[ASTMatchers] AST matcher support for ObjC pointers #117021

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

t-rasmud
Copy link
Contributor

@t-rasmud t-rasmud commented Nov 20, 2024

Add ObjCInterfaceDecl to the list of types supported by the hasType and hasDeclaration matchers, ObjCObjectPointerType to the list of types supported by pointee.
These AST matcher improvements will help the new WebKit checker for unsafe casts (#114606) match on unsafe Objective-C pointer casts.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Nov 20, 2024
@llvmbot
Copy link
Member

llvmbot commented Nov 20, 2024

@llvm/pr-subscribers-clang

Author: Rashmi Mudduluru (t-rasmud)

Changes

Add ObjCInterfaceDecl to the list of types supported by the hasType and hasDeclaration matchers, ObjCObjectPointerType to the list of types supported by pointee.


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

4 Files Affected:

  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+3-2)
  • (modified) clang/include/clang/ASTMatchers/ASTMatchersInternal.h (+10-1)
  • (modified) clang/lib/ASTMatchers/ASTMatchersInternal.cpp (+1-1)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp (+7)
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 4bcaa953a61af2..dc6d60a1bcd17f 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -4033,7 +4033,7 @@ AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
 AST_POLYMORPHIC_MATCHER_P_OVERLOAD(
     hasType,
     AST_POLYMORPHIC_SUPPORTED_TYPES(Expr, FriendDecl, ValueDecl,
-                                    CXXBaseSpecifier),
+                                    CXXBaseSpecifier, ObjCInterfaceDecl),
     internal::Matcher<Decl>, InnerMatcher, 1) {
   QualType QT = internal::getUnderlyingType(Node);
   if (!QT.isNull())
@@ -7434,7 +7434,8 @@ extern const AstTypeMatcher<RValueReferenceType> rValueReferenceType;
 AST_TYPELOC_TRAVERSE_MATCHER_DECL(
     pointee, getPointee,
     AST_POLYMORPHIC_SUPPORTED_TYPES(BlockPointerType, MemberPointerType,
-                                    PointerType, ReferenceType));
+                                    PointerType, ReferenceType,
+                                    ObjCObjectPointerType));
 
 /// Matches typedef types.
 ///
diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index ab8b146453e761..2c690275dab71f 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -161,6 +161,9 @@ inline QualType getUnderlyingType(const FriendDecl &Node) {
 inline QualType getUnderlyingType(const CXXBaseSpecifier &Node) {
   return Node.getType();
 }
+inline QualType getUnderlyingType(const ObjCInterfaceDecl &Node) {
+  return Node.getTypeForDecl()->getPointeeType();
+}
 
 /// Unifies obtaining a `TypeSourceInfo` from different node types.
 template <typename T,
@@ -1113,6 +1116,12 @@ class HasDeclarationMatcher : public MatcherInterface<T> {
     return matchesDecl(Node.getDecl(), Finder, Builder);
   }
 
+  bool matchesSpecialized(const ObjCInterfaceDecl &Node,
+                          ASTMatchFinder *Finder,
+                          BoundNodesTreeBuilder *Builder) const {
+    return matchesDecl(Node.getCanonicalDecl(), Finder, Builder);
+  }
+
   /// Extracts the operator new of the new call and returns whether the
   /// inner matcher matches on it.
   bool matchesSpecialized(const CXXNewExpr &Node,
@@ -1213,7 +1222,7 @@ using HasDeclarationSupportedTypes =
              ElaboratedType, InjectedClassNameType, LabelStmt, AddrLabelExpr,
              MemberExpr, QualType, RecordType, TagType,
              TemplateSpecializationType, TemplateTypeParmType, TypedefType,
-             UnresolvedUsingType, ObjCIvarRefExpr>;
+             UnresolvedUsingType, ObjCIvarRefExpr, ObjCInterfaceDecl>;
 
 /// A Matcher that allows binding the node it matches to an id.
 ///
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 46dd44e6f2b24f..8def98ff6dc328 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -1097,7 +1097,7 @@ AST_TYPELOC_TRAVERSE_MATCHER_DEF(hasValueType,
 AST_TYPELOC_TRAVERSE_MATCHER_DEF(
     pointee,
     AST_POLYMORPHIC_SUPPORTED_TYPES(BlockPointerType, MemberPointerType,
-                                    PointerType, ReferenceType));
+                                    PointerType, ReferenceType, ObjCObjectPointerType));
 
 const internal::VariadicDynCastAllOfMatcher<Stmt, OMPExecutableDirective>
     ompExecutableDirective;
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 1d18869a6b8afc..adf8749a642fc3 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -283,6 +283,13 @@ TEST(HasDeclaration, HasDeclarationOfTypeAlias) {
           hasDeclaration(typeAliasTemplateDecl()))))))));
 }
 
+TEST(HasDeclaration, HasDeclarationOfObjCInterface) {
+  EXPECT_TRUE(matchesObjC(
+      "@interface BaseClass @end void f() {BaseClass* b;}",
+      varDecl(hasType(objcObjectPointerType(pointee(hasDeclaration(
+          objcInterfaceDecl())))))));
+}
+
 TEST(HasUnqualifiedDesugaredType, DesugarsUsing) {
   EXPECT_TRUE(
       matches("struct A {}; using B = A; B b;",

Copy link

github-actions bot commented Nov 20, 2024

✅ With the latest revision this PR passed the C/C++ code formatter.

@t-rasmud t-rasmud requested a review from haoNoQ November 20, 2024 18:48
Copy link
Contributor

@ziqingluo-90 ziqingluo-90 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neat!

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes LGTM as far as they go, but can you add details to the patch summary about why these should be exposed? (We typically only add to the AST matchers when there's an in-tree need for the functionality, so are there checks being updated to make use of these new interfaces or is this more of a change for completeness?)

Also, please add a release note to clang/docs/ReleaseNotes.rst so users know about the feature.

@t-rasmud
Copy link
Contributor Author

The changes LGTM as far as they go, but can you add details to the patch summary about why these should be exposed? (We typically only add to the AST matchers when there's an in-tree need for the functionality, so are there checks being updated to make use of these new interfaces or is this more of a change for completeness?)

Also, please add a release note to clang/docs/ReleaseNotes.rst so users know about the feature.

Thanks for the feedback @AaronBallman. I've updated the release notes and the PR summary.

@t-rasmud
Copy link
Contributor Author

t-rasmud commented Dec 2, 2024

Friendly @ping for approval/feedback.

Copy link
Collaborator

@AaronBallman AaronBallman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@t-rasmud
Copy link
Contributor Author

t-rasmud commented Dec 3, 2024

Thanks for the review!

@t-rasmud t-rasmud force-pushed the ast-matcher-objc-ptr branch from 12ea148 to f624a90 Compare December 3, 2024 18:06
Add `ObjCInterfaceDecl` to the list of types supported by the `hasType` and `hasDeclaration` matchers,
`ObjCObjectPointerType` to the list of types supported by `pointee`.
@t-rasmud t-rasmud force-pushed the ast-matcher-objc-ptr branch from f624a90 to 8cbc275 Compare December 3, 2024 19:12
@t-rasmud t-rasmud merged commit fdd09e9 into llvm:main Dec 3, 2024
9 checks passed
@llvm-ci
Copy link
Collaborator

llvm-ci commented Dec 3, 2024

LLVM Buildbot has detected a new failure on builder clang-armv8-quick running on linaro-clang-armv8-quick while building clang at step 5 "ninja check 1".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/154/builds/8421

Here is the relevant piece of the build log for the reference
Step 5 (ninja check 1) failure: stage 1 checked (failure)
******************** TEST 'Clangd Unit Tests :: ./ClangdTests/251/318' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd Unit Tests-149607-251-318.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=318 GTEST_SHARD_INDEX=251 /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests
--

Note: This is test shard 252 of 318.
[==========] Running 4 tests from 4 test suites.
[----------] Global test environment set-up.
[----------] 1 test from CommandMangler
[ RUN      ] CommandMangler.Sysroot
[       OK ] CommandMangler.Sysroot (305 ms)
[----------] 1 test from CommandMangler (305 ms total)

[----------] 1 test from GlobalCompilationDatabaseTest
[ RUN      ] GlobalCompilationDatabaseTest.BuildDir
Failed to find compilation database for /clangd-test/x/foo.cc
Loaded compilation database from /clangd-test/x/build/compile_commands.json
Failed to find compilation database for /clangd-test/bar.cc
[       OK ] GlobalCompilationDatabaseTest.BuildDir (119 ms)
[----------] 1 test from GlobalCompilationDatabaseTest (119 ms total)

[----------] 1 test from CrossFileRenameTests
[ RUN      ] CrossFileRenameTests.WithUpToDateIndex
ASTWorker building file /clangd-test/foo.h version null with command 
[/clangd-test]
clang -xobjective-c++ /clangd-test/foo.h
Driver produced command: cc1 -cc1 -triple armv8a-unknown-linux-gnueabihf -fsyntax-only -disable-free -clear-ast-before-backend -main-file-name foo.h -mrelocation-model pic -pic-level 2 -pic-is-pie -mframe-pointer=all -fmath-errno -ffp-contract=on -fno-rounding-math -mconstructor-aliases -target-cpu generic -target-feature +vfp2 -target-feature +vfp2sp -target-feature +vfp3 -target-feature +vfp3d16 -target-feature +vfp3d16sp -target-feature +vfp3sp -target-feature +fp16 -target-feature +vfp4 -target-feature +vfp4d16 -target-feature +vfp4d16sp -target-feature +vfp4sp -target-feature +fp-armv8 -target-feature +fp-armv8d16 -target-feature +fp-armv8d16sp -target-feature +fp-armv8sp -target-feature -fullfp16 -target-feature +fp64 -target-feature +d32 -target-feature +neon -target-feature +sha2 -target-feature +aes -target-feature -fp16fml -target-abi aapcs-linux -mfloat-abi hard -debugger-tuning=gdb -fdebug-compilation-dir=/clangd-test -fcoverage-compilation-dir=/clangd-test -resource-dir lib/clang/20 -internal-isystem lib/clang/20/include -internal-isystem /usr/local/include -internal-externc-isystem /include -internal-externc-isystem /usr/include -fdeprecated-macro -ferror-limit 19 -fno-signed-char -fgnuc-version=4.2.1 -fskip-odr-check-in-gmf -fobjc-runtime=gcc -fobjc-encode-cxx-class-template-spec -fobjc-exceptions -fcxx-exceptions -fexceptions -no-round-trip-args -faddrsig -D__GCC_HAVE_DWARF2_CFI_ASM=1 -x objective-c++ /clangd-test/foo.h
Building first preamble for /clangd-test/foo.h version null
not idle after addDocument
UNREACHABLE executed at ../llvm/clang-tools-extra/clangd/unittests/SyncAPI.cpp:22!
Built preamble of size 414568 for file /clangd-test/foo.h version null in 12.26 seconds
indexed preamble AST for /clangd-test/foo.h version null:
  symbol slab: 0 symbols, 68 bytes
  ref slab: 0 symbols, 0 refs, 72 bytes
  relations slab: 0 relations, 12 bytes
#0 0x016a4018 llvm::sys::PrintStackTrace(llvm::raw_ostream&, int) (/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0xb82018)
#1 0x016a19ec llvm::sys::RunSignalHandlers() (/home/tcwg-buildbot/worker/clang-armv8-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests+0xb7f9ec)
#2 0x016a48ec SignalHandler(int) Signals.cpp:0:0
#3 0xf765d6e0 __default_sa_restorer ./signal/../sysdeps/unix/sysv/linux/arm/sigrestorer.S:67:0
#4 0xf764db06 ./csu/../sysdeps/unix/sysv/linux/arm/libc-do-syscall.S:47:0
#5 0xf768d292 __pthread_kill_implementation ./nptl/pthread_kill.c:44:76
#6 0xf765c840 gsignal ./signal/../sysdeps/posix/raise.c:27:6

--
exit: -6
--
shard JSON output does not exist: /home/tcwg-buildbot/worker/clang-armv8-quick/stage1/tools/clang/tools/extra/clangd/unittests/./ClangdTests-Clangd Unit Tests-149607-251-318.json
********************


rniwa pushed a commit to rniwa/llvm-project that referenced this pull request Feb 3, 2025
Add `ObjCInterfaceDecl` to the list of types supported by the `hasType`
and `hasDeclaration` matchers, `ObjCObjectPointerType` to the list of
types supported by `pointee`.
These AST matcher improvements will help the new WebKit checker for
unsafe casts
([https://github.com/llvm/llvm-project/pull/114606](https://github.com/llvm/llvm-project/pull/114606))
match on unsafe Objective-C pointer casts.
devincoughlin pushed a commit to swiftlang/llvm-project that referenced this pull request Feb 25, 2025
Add `ObjCInterfaceDecl` to the list of types supported by the `hasType`
and `hasDeclaration` matchers, `ObjCObjectPointerType` to the list of
types supported by `pointee`.
These AST matcher improvements will help the new WebKit checker for
unsafe casts
([https://github.com/llvm/llvm-project/pull/114606](https://github.com/llvm/llvm-project/pull/114606))
match on unsafe Objective-C pointer casts.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants