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][HeuristicResolver] Only perform qualifier check for instance methods #125166

Merged
merged 1 commit into from
Jan 31, 2025

Conversation

HighCommander4
Copy link
Collaborator

Fixes #125164

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Jan 31, 2025
@llvmbot
Copy link
Member

llvmbot commented Jan 31, 2025

@llvm/pr-subscribers-clang

Author: Nathan Ridge (HighCommander4)

Changes

Fixes #125164


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

2 Files Affected:

  • (modified) clang/lib/Sema/HeuristicResolver.cpp (+2-1)
  • (modified) clang/unittests/Sema/HeuristicResolverTest.cpp (+20)
diff --git a/clang/lib/Sema/HeuristicResolver.cpp b/clang/lib/Sema/HeuristicResolver.cpp
index 0c57250e63df2e6..36e5b44b8b12cc1 100644
--- a/clang/lib/Sema/HeuristicResolver.cpp
+++ b/clang/lib/Sema/HeuristicResolver.cpp
@@ -482,7 +482,8 @@ std::vector<const NamedDecl *> HeuristicResolverImpl::resolveDependentMember(
       if (!Filter(ND))
         return false;
       if (const auto *MD = dyn_cast<CXXMethodDecl>(ND)) {
-        return MD->getMethodQualifiers().compatiblyIncludes(QT.getQualifiers(),
+        return !MD->isInstance() ||
+               MD->getMethodQualifiers().compatiblyIncludes(QT.getQualifiers(),
                                                             Ctx);
       }
       return true;
diff --git a/clang/unittests/Sema/HeuristicResolverTest.cpp b/clang/unittests/Sema/HeuristicResolverTest.cpp
index a0deb2d936756af..5c3459dbeb1018b 100644
--- a/clang/unittests/Sema/HeuristicResolverTest.cpp
+++ b/clang/unittests/Sema/HeuristicResolverTest.cpp
@@ -155,6 +155,26 @@ TEST(HeuristicResolver, MemberExpr_SmartPointer_Qualified) {
       cxxMethodDecl(hasName("find"), isConst()).bind("output"));
 }
 
+TEST(HeuristicResolver, MemberExpr_Static_Qualified) {
+  std::string Code = R"cpp(
+    template <typename T>
+    struct Waldo {
+      static void find();
+    };
+    template <typename T>
+    void foo(const Waldo<T>& t) {
+      t.find();
+    }
+  )cpp";
+  // Test resolution of "find" in "t.find()".
+  // The object being `const` should have no bearing on a call to a static
+  // method.
+  expectResolution(
+      Code, &HeuristicResolver::resolveMemberExpr,
+      cxxDependentScopeMemberExpr(hasMemberName("find")).bind("input"),
+      cxxMethodDecl(hasName("find")).bind("output"));
+}
+
 TEST(HeuristicResolver, MemberExpr_AutoTypeDeduction1) {
   std::string Code = R"cpp(
     template <typename T>

@HighCommander4
Copy link
Collaborator Author

Part of me wonders if there's ever a legitimate reason to call CXXMethodDecl::getMethodQualifiers() on a static method...

Copy link
Contributor

@zyn0217 zyn0217 left a comment

Choose a reason for hiding this comment

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

Oops...

@HighCommander4 HighCommander4 merged commit 2941fa3 into main Jan 31, 2025
11 checks passed
@HighCommander4 HighCommander4 deleted the users/HighCommander4/issue-125164 branch January 31, 2025 05:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

HeuristicResolver incorrectly applies const-checking to static methods called through an instance
3 participants