Skip to content

[clang][test] remove unused run overload in BoundNodesCallback #105935

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

Conversation

5chmidti
Copy link
Contributor

The overload that did not take the additional ASTContext * argument is unnecessary when the context could simply be commented out, as it is always passed to run from VerifyMatcher::run.
This patch removes the single-argument overload in favor of having a single overload.

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

llvmbot commented Aug 24, 2024

@llvm/pr-subscribers-clang

Author: Julian Schmidt (5chmidti)

Changes

The overload that did not take the additional ASTContext * argument is unnecessary when the context could simply be commented out, as it is always passed to run from VerifyMatcher::run.
This patch removes the single-argument overload in favor of having a single overload.


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

3 Files Affected:

  • (modified) clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp (-2)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersTest.h (+1-6)
  • (modified) clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp (+4-11)
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index 3295ad1e21455f..ebf548eb254313 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2030,8 +2030,6 @@ TEST_P(ASTMatchersTest,
 template <typename T>
 class VerifyAncestorHasChildIsEqual : public BoundNodesCallback {
 public:
-  bool run(const BoundNodes *Nodes) override { return false; }
-
   bool run(const BoundNodes *Nodes, ASTContext *Context) override {
     const T *Node = Nodes->getNodeAs<T>("");
     return verify(*Nodes, *Context, Node);
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTest.h b/clang/unittests/ASTMatchers/ASTMatchersTest.h
index e9812995315741..ad2f5f355621cd 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTest.h
+++ b/clang/unittests/ASTMatchers/ASTMatchersTest.h
@@ -28,7 +28,6 @@ using clang::tooling::runToolOnCodeWithArgs;
 class BoundNodesCallback {
 public:
   virtual ~BoundNodesCallback() {}
-  virtual bool run(const BoundNodes *BoundNodes) = 0;
   virtual bool run(const BoundNodes *BoundNodes, ASTContext *Context) = 0;
   virtual void onEndOfTranslationUnit() {}
 };
@@ -403,7 +402,7 @@ template <typename T> class VerifyIdIsBoundTo : public BoundNodesCallback {
     EXPECT_EQ("", Name);
   }
 
-  bool run(const BoundNodes *Nodes) override {
+  bool run(const BoundNodes *Nodes, ASTContext * /*Context*/) override {
     const BoundNodes::IDToNodeMap &M = Nodes->getMap();
     if (Nodes->getNodeAs<T>(Id)) {
       ++Count;
@@ -426,10 +425,6 @@ template <typename T> class VerifyIdIsBoundTo : public BoundNodesCallback {
     return false;
   }
 
-  bool run(const BoundNodes *Nodes, ASTContext *Context) override {
-    return run(Nodes);
-  }
-
 private:
   const std::string Id;
   const int ExpectedCount;
diff --git a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
index 8a62358a71f0bf..b91eaf91c0bf23 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -5653,7 +5653,6 @@ TEST(HasParent, MatchesAllParents) {
 TEST(HasParent, NoDuplicateParents) {
   class HasDuplicateParents : public BoundNodesCallback {
   public:
-    bool run(const BoundNodes *Nodes) override { return false; }
     bool run(const BoundNodes *Nodes, ASTContext *Context) override {
       const Stmt *Node = Nodes->getNodeAs<Stmt>("node");
       std::set<const void *> Parents;
@@ -5862,16 +5861,14 @@ template <typename T> class VerifyMatchOnNode : public BoundNodesCallback {
 public:
   VerifyMatchOnNode(StringRef Id, const internal::Matcher<T> &InnerMatcher,
                     StringRef InnerId)
-    : Id(Id), InnerMatcher(InnerMatcher), InnerId(InnerId) {
-  }
-
-  bool run(const BoundNodes *Nodes) override { return false; }
+      : Id(Id), InnerMatcher(InnerMatcher), InnerId(InnerId) {}
 
   bool run(const BoundNodes *Nodes, ASTContext *Context) override {
     const T *Node = Nodes->getNodeAs<T>(Id);
     return selectFirst<T>(InnerId, match(InnerMatcher, *Node, *Context)) !=
-      nullptr;
+           nullptr;
   }
+
 private:
   std::string Id;
   internal::Matcher<T> InnerMatcher;
@@ -6065,7 +6062,7 @@ namespace {
 class ForCallablePreservesBindingWithMultipleParentsTestCallback
     : public BoundNodesCallback {
 public:
-  bool run(const BoundNodes *BoundNodes) override {
+  bool run(const BoundNodes *BoundNodes, ASTContext *Context) override {
     FunctionDecl const *FunDecl =
         BoundNodes->getNodeAs<FunctionDecl>("funDecl");
     // Validate test assumptions. This would be expressed as ASSERT_* in
@@ -6102,10 +6099,6 @@ class ForCallablePreservesBindingWithMultipleParentsTestCallback
     return true;
   }
 
-  bool run(const BoundNodes *BoundNodes, ASTContext *Context) override {
-    return run(BoundNodes);
-  }
-
 private:
   void ExpectCorrectResult(StringRef LogInfo,
                            ArrayRef<BoundNodes> Results) const {

@5chmidti
Copy link
Contributor Author

Recreated after accidental merge in #94244 because this is part of a stack.
Previous approval is in the linked pr.

@5chmidti 5chmidti force-pushed the users/5chmidti/rm_not_needed_run_overload_in_BoundNodesCallback branch 2 times, most recently from 2072160 to d4d44d6 Compare August 29, 2024 15:36
Base automatically changed from users/5chmidti/specify_test_language_versions_in_def_file to main September 27, 2024 11:03
The overload that did not take the additional `ASTContext *` argument is
unnecessary when the context could simply be commented out, as it is
always passed to `run` from `VerifyMatcher::run`.
This patch removes the single-argument overload in favor of having a
single overload.
@5chmidti 5chmidti force-pushed the users/5chmidti/rm_not_needed_run_overload_in_BoundNodesCallback branch from d4d44d6 to 388be9e Compare September 27, 2024 11:23
@5chmidti 5chmidti merged commit d5dc508 into main Sep 27, 2024
5 of 7 checks passed
@5chmidti 5chmidti deleted the users/5chmidti/rm_not_needed_run_overload_in_BoundNodesCallback branch September 27, 2024 11:23
Sterling-Augustine pushed a commit to Sterling-Augustine/llvm-project that referenced this pull request Sep 27, 2024
…lvm#105935)

The overload that did not take the additional `ASTContext *` argument is
unnecessary when the context could simply be commented out, as it is
always passed to `run` from `VerifyMatcher::run`.
This patch removes the single-argument overload in favor of having a
single overload.
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.

2 participants