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

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

Open
wants to merge 1 commit into
base: users/5chmidti/specify_test_language_versions_in_def_file
Choose a base branch
from

Conversation

5chmidti
Copy link
Contributor

@5chmidti 5chmidti commented Jun 3, 2024

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 Jun 3, 2024
@llvmbot
Copy link
Collaborator

llvmbot commented Jun 3, 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/94244.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 (+3-6)
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index aed6a6408adc9..dd98fbdce3945 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 e981299531574..ad2f5f355621c 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 ada3be287ed59..6bb402caa4d6d 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersTraversalTest.cpp
@@ -5641,7 +5641,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;
@@ -5850,16 +5849,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;

@5chmidti 5chmidti force-pushed the users/5chmidti/specify_test_language_versions_in_def_file branch from ab818f8 to 1c6f2a0 Compare June 3, 2024 16:36
@5chmidti 5chmidti force-pushed the users/5chmidti/rm_not_needed_run_overload_in_BoundNodesCallback branch from 0c53f15 to 615f30b Compare June 3, 2024 16:37
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!

@5chmidti 5chmidti force-pushed the users/5chmidti/specify_test_language_versions_in_def_file branch from 1c6f2a0 to 1383ace Compare July 12, 2024 21:59
@5chmidti 5chmidti force-pushed the users/5chmidti/rm_not_needed_run_overload_in_BoundNodesCallback branch from 615f30b to 51de627 Compare July 12, 2024 22:00
@5chmidti
Copy link
Contributor Author

rebase on trunk + rebased stack

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 51de627 to 26d5b03 Compare July 12, 2024 23:13
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

3 participants