[clang] Include explicit object methods in overload suggestions #154041

MythreyaK · 2025-08-17T22:50:49Z

Include methods that use explicit object in code complete suggestions.

struct S {
  void foo1(int a) const;
  void foo2(int a);
  void foo2(this const S& self, float a); // shows up as overload of foo2
  void foo3(this const S& self, int a);
  void foo4(this S& self, int a);
};

int foo(const S arg) {
  arg.f^ // Now suggests foo3 as well 
}

clang/include/clang/AST/DeclCXX.h

MythreyaK · 2025-08-17T22:54:30Z

clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp

Test slightly modified from example that @zwuis originally shared in the ticket.

Happy to change or update!

MythreyaK · 2025-08-18T03:20:18Z

clang/lib/Sema/SemaCodeComplete.cpp

Using a lambda for now, but we can probably reuse this in other functions as well. Not 100% sure if getFunctionObjectParameterType().getQualifiers() is the right way to check 🤔.

FWIW, I tried to evaluate whether it would be appropriate for CXXMethodDecl::getMethodQualifiers() itself to behave this way, but the code at the call sites of that method in actual semantic analysis code is going a bit over my head. I think keeping this logic local to this call site is fine for now.

I think changing CXXMethodDecl::getMethodQualifiers() would break name mangling.

Tried it out, here's the diff (b/w this and the new PR) and the corresponding draft PR.

llvmbot · 2025-08-18T08:46:33Z

@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clangd

@llvm/pr-subscribers-clang-tools-extra

Author: Mythreya Kuricheti (MythreyaK)

Changes

Fix for #109608.

Draft to gather feedback.

Signature help still needs fixing--signature for void foo3(this const S& self) is shown as (int a) instead of (int a) const. ~~Will add a commit soon.~~ Done ✔️.

Include methods that use explicit object in code complete suggestions.

struct S {
  void foo1() const;
  void foo2();
  void foo3(this const S&amp; self);
  void foo4(this S&amp; self);
};

int foo(const S arg) {
  arg.f^ // Now suggests foo3 as well 
}

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

2 Files Affected:

(modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+119)
(modified) clang/lib/Sema/SemaCodeComplete.cpp (+36-1)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 1a1c32c241602..cf07e11d6441e 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4473,6 +4473,125 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
                                   snippetSuffix(""))));
   }
 }
+
+TEST(CompletionTest, ListExplicitObjectOverloads) {
+  Annotations Code(R"cpp(
+    struct S {
+      void foo1(int a);
+      void foo2(int a) const;
+      void foo3(this const S& self, int a);
+      void foo4(this S& self, int a);
+    };
+
+    void S::foo1(int a) {
+      this->$c1^;
+    }
+
+    void S::foo2(int a) const {
+      this->$c2^;
+    }
+
+    void S::foo3(this const S& self, int a) {
+      self.$c3^;
+    }
+
+    void S::foo4(this S& self, int a) {
+      self.$c4^;
+    }
+
+    void test1(S s) {
+      s.$c5^;
+    }
+
+    void test2(const S s) {
+      s.$c6^;
+    }
+  )cpp");
+
+  auto TU = TestTU::withCode(Code.code());
+  TU.ExtraArgs = {"-std=c++23"};
+
+  auto Preamble = TU.preamble();
+  ASSERT_TRUE(Preamble);
+
+  CodeCompleteOptions Opts{};
+
+  MockFS FS;
+  auto Inputs = TU.inputs(FS);
+
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c1"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        UnorderedElementsAre(AllOf(named("foo1"), signature("(int a)"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo2"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo3"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo4"), signature("(int a)"),
+                                   snippetSuffix("(${1:int a})"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c2"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        UnorderedElementsAre(AllOf(named("foo2"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo3"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c3"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        UnorderedElementsAre(AllOf(named("foo2"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo3"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c4"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        UnorderedElementsAre(AllOf(named("foo1"), signature("(int a)"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo2"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo3"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo4"), signature("(int a)"),
+                                   snippetSuffix("(${1:int a})"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c5"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        UnorderedElementsAre(AllOf(named("foo1"), signature("(int a)"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo2"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo3"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo4"), signature("(int a)"),
+                                   snippetSuffix("(${1:int a})"))));
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c6"),
+                               Preamble.get(), Inputs, Opts);
+    EXPECT_THAT(
+        Result.Completions,
+        UnorderedElementsAre(AllOf(named("foo2"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})")),
+                             AllOf(named("foo3"), signature("(int a) const"),
+                                   snippetSuffix("(${1:int a})"))));
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index e4f276086af25..af900c777a707 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -1428,10 +1428,18 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
 
   AdjustResultPriorityForDecl(R);
 
+  // Account for explicit object parameter
+  const auto getQualifiers = [&](const CXXMethodDecl *MethodDecl) {
+    if (MethodDecl->isExplicitObjectMemberFunction())
+      return MethodDecl->getFunctionObjectParameterType().getQualifiers();
+    else
+      return MethodDecl->getMethodQualifiers();
+  };
+
   if (HasObjectTypeQualifiers)
     if (const auto *Method = dyn_cast<CXXMethodDecl>(R.Declaration))
       if (Method->isInstance()) {
-        Qualifiers MethodQuals = Method->getMethodQualifiers();
+        Qualifiers MethodQuals = getQualifiers(Method);
         if (ObjectTypeQualifiers == MethodQuals)
           R.Priority += CCD_ObjectQualifierMatch;
         else if (ObjectTypeQualifiers - MethodQuals) {
@@ -3410,9 +3418,36 @@ static void AddQualifierToCompletionString(CodeCompletionBuilder &Result,
     Result.AddTextChunk(Result.getAllocator().CopyString(PrintedNNS));
 }
 
+// Sets the function qualifiers completion string by inspecting the explicit
+// object
+static void AddCXXExplicitObjectFunctionTypeQualsToCompletionString(
+    CodeCompletionBuilder &Result, const CXXMethodDecl *Function) {
+  const auto Quals = Function->getFunctionObjectParameterType();
+
+  if (!Quals.hasQualifiers())
+    return;
+
+  std::string QualsStr;
+  if (Quals.getQualifiers().hasConst())
+    QualsStr += " const";
+  if (Quals.getQualifiers().hasVolatile())
+    QualsStr += " volatile";
+  if (Quals.getQualifiers().hasRestrict())
+    QualsStr += " restrict";
+  Result.AddInformativeChunk(Result.getAllocator().CopyString(QualsStr));
+}
+
 static void
 AddFunctionTypeQualsToCompletionString(CodeCompletionBuilder &Result,
                                        const FunctionDecl *Function) {
+  if (auto *CxxMethodDecl = llvm::dyn_cast_if_present<CXXMethodDecl>(Function);
+      CxxMethodDecl && CxxMethodDecl->hasCXXExplicitFunctionObjectParameter()) {
+    // if explicit object method, infer quals from the object parameter
+    AddCXXExplicitObjectFunctionTypeQualsToCompletionString(Result,
+                                                            CxxMethodDecl);
+    return;
+  }
+
   const auto *Proto = Function->getType()->getAs<FunctionProtoType>();
   if (!Proto || !Proto->getMethodQuals())
     return;

MythreyaK · 2025-08-18T09:31:08Z

clang/test/CodeCompletion/skip-explicit-object-parameter.cpp

Need to check why foo2 is broken. But autocomplete seems to work fine 🤔

foo2(float a) vs [incorrect?] foo2(<#float a#>) [expected?]

HighCommander4 · 2025-08-19T04:05:33Z

clang/lib/Sema/SemaCodeComplete.cpp

nit: as a local variable, the lambda name should be UpperCamelCase (I know this is counterintuitive)

Wasn't sure which way to go here 😆, done!

HighCommander4 · 2025-08-20T05:45:53Z

clang/lib/Sema/SemaCodeComplete.cpp

This is a nice touch!

One minor comment: could we refactor this to reuse code between this function, and the rest of AddFunctionTypeQualsToCompletionString? (That way, the "Handle single qualifiers without copying" optimization would apply here as well.)

After the refactor for code-reuse, it feels like the body of AddCXXExplicitObjectFunctionTypeQualsToCompletionString can be moved back into AddFunctionTypeQualsToCompletionString, to make it similar to the code in else branch.

I think either is fine, leaving this as is or moving the body back into AddFunctionTypeQualsToCompletionString, let me know what you think.

Moving it back seems like it might be a bit nicer

HighCommander4 · 2025-08-20T05:52:11Z

clang/lib/Sema/SemaCodeComplete.cpp

FWIW, I tried to evaluate whether it would be appropriate for CXXMethodDecl::getMethodQualifiers() itself to behave this way, but the code at the call sites of that method in actual semantic analysis code is going a bit over my head. I think keeping this logic local to this call site is fine for now.

HighCommander4

Thanks for the update, just minor comments remaining and then I think this is good to merge.

HighCommander4 · 2025-08-21T05:11:02Z

clang/lib/Sema/SemaCodeComplete.cpp

nit: can we move this closer to where it's used? (we can make it static if needed)

HighCommander4 · 2025-08-21T05:11:38Z

clang/lib/Sema/SemaCodeComplete.cpp

Moving it back seems like it might be a bit nicer

MythreyaK · 2025-08-21T05:33:12Z

Thanks for the review!

I have 2 remaining questions,

With this PR, these 2 show up as overloads, though not sure if they should be considered overloads. It's convenient, but wanted to double-check.
```
void foo2(int a);
void foo2(this const S& self, float a); // shows up as overload of foo2
```
There seems to be an issue with the lit tests, described here. Is this a problem?

HighCommander4

LGTM

HighCommander4 · 2025-08-21T06:10:29Z

With this PR, these 2 show up as overloads, though not sure if they should be considered overloads. It's convenient, but wanted to double-check.
void foo2(int a);
void foo2(this const S& self, float a); // shows up as overload of foo2

I think that makes sense; from the point of view of calling code they're overloads, and the fact that one uses a this parameter and one doesn't is kind of an implementation detail.

There seems to be an issue with the lit tests, described here. Is this a problem?

It's mildly suspicious, but I would say not worrying enough to hold up this patch. Feel free to file a follow-up issue about the FIXME so it's not forgotten.

MythreyaK commented Aug 17, 2025

View reviewed changes

clang/include/clang/AST/DeclCXX.h Outdated Show resolved Hide resolved

MythreyaK commented Aug 17, 2025

View reviewed changes

MythreyaK force-pushed the mythreyak/cc-this-members branch from d730c1f to 8df8893 Compare August 18, 2025 03:18

MythreyaK commented Aug 18, 2025

View reviewed changes

MythreyaK force-pushed the mythreyak/cc-this-members branch 3 times, most recently from ae6d92a to 9f149ac Compare August 18, 2025 08:18

MythreyaK marked this pull request as ready for review August 18, 2025 08:46

llvmbot added clang Clang issues not falling into any other category clang-tools-extra clangd clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Aug 18, 2025

MythreyaK force-pushed the mythreyak/cc-this-members branch 2 times, most recently from 524cde5 to 304ab86 Compare August 18, 2025 09:27

MythreyaK commented Aug 18, 2025

View reviewed changes

HighCommander4 requested changes Aug 20, 2025

View reviewed changes

MythreyaK force-pushed the mythreyak/cc-this-members branch 2 times, most recently from 2946340 to 83c35d9 Compare August 20, 2025 10:19

MythreyaK mentioned this pull request Aug 20, 2025

Draft #154505

Closed

MythreyaK added 4 commits August 20, 2025 03:49

[clang] Include explicit object methods in overload suggestions

471fa8c

Add const to signature string

aa2c6fe

Update tests

639da07

code review

d2476ed

HighCommander4 approved these changes Aug 21, 2025

View reviewed changes

MythreyaK force-pushed the mythreyak/cc-this-members branch from 83c35d9 to d2476ed Compare August 21, 2025 05:32

code review

888ef93

HighCommander4 approved these changes Aug 21, 2025

View reviewed changes

MythreyaK mentioned this pull request Aug 21, 2025

Suspicious lit test output for function parameter #154676

Open

HighCommander4 merged commit 0977a6d into llvm:main Aug 21, 2025
9 checks passed

MythreyaK deleted the mythreyak/cc-this-members branch August 21, 2025 06:34

[clang] Include explicit object methods in overload suggestions #154041

[clang] Include explicit object methods in overload suggestions #154041

Uh oh!

Conversation

MythreyaK commented Aug 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

llvmbot commented Aug 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

HighCommander4 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MythreyaK commented Aug 21, 2025

Uh oh!

HighCommander4 left a comment

Choose a reason for hiding this comment

Uh oh!

HighCommander4 commented Aug 21, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MythreyaK commented Aug 17, 2025 •

edited

Loading

llvmbot commented Aug 18, 2025 •

edited

Loading