Skip to content
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

Add 'forNone' and 'forNoDescendant' AST matchers #86230

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

hach-que
Copy link

@hach-que hach-que commented Mar 22, 2024

This adds forNone and forNoDescendant AST matchers. forNoDescendant is simply the recursive version of forNone.

forNone matches only if there are no immediate children of the current node that match the inner matcher. For example, given:

class F
{
public:
  int A;

  F() {};
};

the matcher:

cxxConstructorDecl(
    unless(isImplicit()),
    unless(isDelegatingConstructor()),
    unless(isDeleted()),
    unless(isDefaulted()),
    hasBody(stmt()),
    ofClass(cxxRecordDecl(forEach(fieldDecl().bind("declared_field")))),
    forNone(cxxCtorInitializer(forField(fieldDecl(equalsBoundNode("declared_field")).bind("referenced_field"))))
).bind("bad_constructor")

would match F(), because it does not have an initializer for A. We use this in our modified version of Clang to detect constructors that do not fully initialize all fields.

Note: The forNoDescendant AST matcher is included in this pull request, as both matchers depend on the new BoundNodesTreeBuilder::contains function.

Copy link

Thank you for submitting a Pull Request (PR) to the LLVM Project!

This PR will be automatically labeled and the relevant teams will be
notified.

If you wish to, you can add reviewers by using the "Reviewers" section on this page.

If this is not working for you, it is probably because you do not have write
permissions for the repository. In which case you can instead tag reviewers by
name in a comment by using @ followed by their GitHub username.

If you have received no comments on your PR for a week, you can request a review
by "ping"ing the PR by adding a comment “Ping”. The common courtesy "ping" rate
is once a week. Please remember that you are asking for valuable time from other developers.

If you have further questions, they may be answered by the LLVM GitHub User Guide.

You can also ask questions in a comment on this PR, on the LLVM Discord or on the forums.

@llvmbot llvmbot added the clang Clang issues not falling into any other category label Mar 22, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Mar 22, 2024

@llvm/pr-subscribers-clang

Author: June Rhodes (hach-que)

Changes

This adds a forNone AST matcher, which matches only if there are no immediate children of the current node that match the inner matcher. For example, given:

class F
{
public:
  int A;

  F() {};
};

the matcher:

cxxConstructorDecl(
    unless(isImplicit()),
    unless(isDelegatingConstructor()),
    unless(isDeleted()),
    unless(isDefaulted()),
    hasBody(stmt()),
    ofClass(cxxRecordDecl(forEach(fieldDecl().bind("declared_field")))),
    forNone(cxxCtorInitializer(forField(fieldDecl(equalsBoundNode("declared_field")).bind("referenced_field"))))
).bind("bad_constructor")

would match F(), because it does not have an initializer for A. We use this in our modified version of Clang to detect constructors that do not fully initialize all fields.


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

4 Files Affected:

  • (modified) clang/include/clang/ASTMatchers/ASTMatchers.h (+7)
  • (modified) clang/include/clang/ASTMatchers/ASTMatchersInternal.h (+36)
  • (modified) clang/lib/ASTMatchers/ASTMatchersInternal.cpp (+2)
  • (modified) clang/lib/ASTMatchers/Dynamic/Registry.cpp (+1)
diff --git a/clang/include/clang/ASTMatchers/ASTMatchers.h b/clang/include/clang/ASTMatchers/ASTMatchers.h
index 2f71053d030f68..8cc7a0e92acbdd 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -3547,6 +3547,13 @@ extern const internal::ArgumentAdaptingMatcherFunc<
     internal::ForEachDescendantMatcher>
     forEachDescendant;
 
+/// Matches AST nodes that have no child AST nodes that match the
+/// provided matcher.
+///
+/// Usable as: Any Matcher
+extern const internal::ArgumentAdaptingMatcherFunc<internal::ForNoneMatcher>
+    forNone;
+
 /// Matches if the node or any descendant matches.
 ///
 /// Generates results for each match.
diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index 47d912c73dd7eb..bf5aaf74c0ef9a 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1626,6 +1626,42 @@ class ForEachMatcher : public MatcherInterface<T> {
   }
 };
 
+/// Matches nodes of type T that have no child nodes of type ChildT for
+/// which a specified child matcher matches. ChildT must be an AST base
+/// type.
+/// ForNoneMatcher will only match if none of the child nodes match
+/// the inner matcher.
+template <typename T, typename ChildT>
+class ForNoneMatcher : public MatcherInterface<T> {
+  static_assert(IsBaseType<ChildT>::value,
+                "for none only accepts base type matcher");
+
+  DynTypedMatcher InnerMatcher;
+
+public:
+  explicit ForNoneMatcher(const Matcher<ChildT> &InnerMatcher)
+      : InnerMatcher(InnerMatcher) {}
+
+  bool matches(const T &Node, ASTMatchFinder *Finder,
+               BoundNodesTreeBuilder *Builder) const override {
+    BoundNodesTreeBuilder MatchingBuilder(*Builder);
+    bool AnyMatched = Finder->matchesChildOf(Node, this->InnerMatcher, &MatchingBuilder,
+                                        ASTMatchFinder::BK_All);
+    if (!AnyMatched) {
+      // We didn't iterate over any nodes that matched, so
+      // Builder would be empty. This is a success case.
+      return true;
+    }
+    // Otherwise remove from Builder any entries that we
+    // also have in MatchingBuilder because we want to leave
+    // only the remaining entries.
+    return Builder->removeBindings(
+        [&MatchingBuilder](const internal::BoundNodesMap &Nodes) {
+          return MatchingBuilder.contains(Nodes);
+        });
+  }
+};
+
 /// @}
 
 template <typename T>
diff --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index bf87b1aa0992a5..4a8f383011b336 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -1022,6 +1022,8 @@ const internal::ArgumentAdaptingMatcherFunc<internal::HasDescendantMatcher>
     hasDescendant = {};
 const internal::ArgumentAdaptingMatcherFunc<internal::ForEachMatcher> forEach =
     {};
+const internal::ArgumentAdaptingMatcherFunc<internal::ForNoneMatcher> forNone =
+    {};
 const internal::ArgumentAdaptingMatcherFunc<internal::ForEachDescendantMatcher>
     forEachDescendant = {};
 const internal::ArgumentAdaptingMatcherFunc<
diff --git a/clang/lib/ASTMatchers/Dynamic/Registry.cpp b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
index 2c75e6beb74301..f6b866e6a0bcbf 100644
--- a/clang/lib/ASTMatchers/Dynamic/Registry.cpp
+++ b/clang/lib/ASTMatchers/Dynamic/Registry.cpp
@@ -259,6 +259,7 @@ RegistryMaps::RegistryMaps() {
   REGISTER_MATCHER(forEachOverridden);
   REGISTER_MATCHER(forEachSwitchCase);
   REGISTER_MATCHER(forEachTemplateArgument);
+  REGISTER_MATCHER(forNone);
   REGISTER_MATCHER(forField);
   REGISTER_MATCHER(forFunction);
   REGISTER_MATCHER(forStmt);

Copy link

github-actions bot commented Mar 22, 2024

⚠️ C/C++ code formatter, clang-format found issues in your code. ⚠️

You can test this locally with the following command:
git-clang-format --diff 718fbbef5f18a2b7e7fc4f842b1452ae9bee581a 25e3b11324ba4fc43e36035d357d1aa785898bbc -- clang/include/clang/ASTMatchers/ASTMatchers.h clang/include/clang/ASTMatchers/ASTMatchersInternal.h clang/lib/ASTMatchers/ASTMatchersInternal.cpp clang/lib/ASTMatchers/Dynamic/Registry.cpp
View the diff from clang-format here.
diff --git a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
index b40f0b77b0..985bdf8248 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchersInternal.h
@@ -1804,7 +1804,7 @@ public:
   bool matches(const T &Node, ASTMatchFinder *Finder,
                BoundNodesTreeBuilder *Builder) const override {
     BoundNodesTreeBuilder MatchingBuilder(*Builder);
-    bool AnyMatched = 
+    bool AnyMatched =
         Finder->matchesDescendantOf(Node, this->DescendantMatcher,
                                     &MatchingBuilder, ASTMatchFinder::BK_All);
     if (!AnyMatched) {

@hach-que hach-que force-pushed the ast-matcher-fornone branch 2 times, most recently from bf78fb2 to 9b04a03 Compare March 22, 2024 02:24
@hach-que hach-que changed the title Add 'forNone' AST matcher Add 'forNone' and 'forNoDescendant' AST matchers Mar 22, 2024
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.

None yet

2 participants