[clang] Skip suggesting unqualified members in explicit-object member functions #153760

MythreyaK · 2025-08-15T07:55:54Z

Skip suggesting members when not using self. in explicit object methods.

MythreyaK · 2025-08-15T09:00:38Z

clang/test/CodeCompletion/cpp23-explicit-object.cpp

Had to add this paren, otherwise test fails

clang/test/CodeCompletion/cpp23-explicit-object.cpp:46:1: error: expected expression 46 | } | ^ 1 error generated.

Renamed this file from skip-explicit-object-parameter.cpp to cpp23-explicit-object.cpp and added the test here, instead of creating a new file.

llvmbot · 2025-08-15T09:09:30Z

@llvm/pr-subscribers-clang

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

Author: Mythreya Kuricheti (MythreyaK)

Changes

Fixes #141291

Skip suggesting members when not using self. in explicit object methods.

[TODO: Need to add a semacodecomplete test too, will add it in a new commit]

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

3 Files Affected:

(modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+39)
(modified) clang/lib/Sema/SemaCodeComplete.cpp (+10)
(renamed) clang/test/CodeCompletion/cpp23-explicit-object.cpp (+17-1)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 1a1c32c241602..360693a02a1f5 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4473,6 +4473,45 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
                                   snippetSuffix(""))));
   }
 }
+
+TEST(CompletionTest, MemberAccessInExplicitObjMemfn) {
+  Annotations Code(R"cpp(
+    struct A {
+      int member {};
+
+      void foo(this A& self) {
+        // Should not offer `member` here, since it needs to be 
+        // referenced as `self.member`.
+        mem$c1^;
+        self.mem$c2^;
+      }
+    };
+  )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, ElementsAre());
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c2"),
+                               Preamble.get(), Inputs, Opts);
+
+    EXPECT_THAT(Result.Completions, ElementsAre(named("member")));
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index e4f276086af25..224d105c313e6 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -1428,6 +1428,16 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
 
   AdjustResultPriorityForDecl(R);
 
+  if (isa<FieldDecl>(R.Declaration)) {
+    // If result is a member in the context of an explicit-object member
+    // function, drop it because it must be accessed through the object
+    // parameter
+    if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(CurContext);
+        MethodDecl && MethodDecl->isExplicitObjectMemberFunction()) {
+      return;
+    }
+  }
+
   if (HasObjectTypeQualifiers)
     if (const auto *Method = dyn_cast<CXXMethodDecl>(R.Declaration))
       if (Method->isInstance()) {
diff --git a/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp b/clang/test/CodeCompletion/cpp23-explicit-object.cpp
similarity index 75%
rename from clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
rename to clang/test/CodeCompletion/cpp23-explicit-object.cpp
index 587d6cb044d19..c903e2dbbf0a9 100644
--- a/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
+++ b/clang/test/CodeCompletion/cpp23-explicit-object.cpp
@@ -42,7 +42,23 @@ int func3() {
 
 int func4() {
   // TODO (&A::foo)(
-  (&A::bar)(
+  (&A::bar)()
 }
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-2):13 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC5 %s
 // CHECK-CC5: OVERLOAD: [#void#](<#A#>, int)
+
+struct C {
+  int member {};
+  void foo(this C& self) {
+    // Should not offer `member` here, since it needs to be 
+    // referenced as `self.member`.
+    mem
+  }
+  void bar(this C& self) {
+    self.mem
+  }
+};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-8):8 -std=c++23 %s | FileCheck --allow-empty %s
+// CHECK-NOT: COMPLETION: member : [#int#]member
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-5):13 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC6 %s
+// CHECK-CC6: COMPLETION: member : [#int#]member

llvmbot · 2025-08-15T09:09:30Z

@llvm/pr-subscribers-clangd

Author: Mythreya Kuricheti (MythreyaK)

Changes

Fixes #141291

Skip suggesting members when not using self. in explicit object methods.

[TODO: Need to add a semacodecomplete test too, will add it in a new commit]

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

3 Files Affected:

(modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+39)
(modified) clang/lib/Sema/SemaCodeComplete.cpp (+10)
(renamed) clang/test/CodeCompletion/cpp23-explicit-object.cpp (+17-1)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 1a1c32c241602..360693a02a1f5 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4473,6 +4473,45 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
                                   snippetSuffix(""))));
   }
 }
+
+TEST(CompletionTest, MemberAccessInExplicitObjMemfn) {
+  Annotations Code(R"cpp(
+    struct A {
+      int member {};
+
+      void foo(this A& self) {
+        // Should not offer `member` here, since it needs to be 
+        // referenced as `self.member`.
+        mem$c1^;
+        self.mem$c2^;
+      }
+    };
+  )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, ElementsAre());
+  }
+  {
+    auto Result = codeComplete(testPath(TU.Filename), Code.point("c2"),
+                               Preamble.get(), Inputs, Opts);
+
+    EXPECT_THAT(Result.Completions, ElementsAre(named("member")));
+  }
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index e4f276086af25..224d105c313e6 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -1428,6 +1428,16 @@ void ResultBuilder::AddResult(Result R, DeclContext *CurContext,
 
   AdjustResultPriorityForDecl(R);
 
+  if (isa<FieldDecl>(R.Declaration)) {
+    // If result is a member in the context of an explicit-object member
+    // function, drop it because it must be accessed through the object
+    // parameter
+    if (auto *MethodDecl = dyn_cast<CXXMethodDecl>(CurContext);
+        MethodDecl && MethodDecl->isExplicitObjectMemberFunction()) {
+      return;
+    }
+  }
+
   if (HasObjectTypeQualifiers)
     if (const auto *Method = dyn_cast<CXXMethodDecl>(R.Declaration))
       if (Method->isInstance()) {
diff --git a/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp b/clang/test/CodeCompletion/cpp23-explicit-object.cpp
similarity index 75%
rename from clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
rename to clang/test/CodeCompletion/cpp23-explicit-object.cpp
index 587d6cb044d19..c903e2dbbf0a9 100644
--- a/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
+++ b/clang/test/CodeCompletion/cpp23-explicit-object.cpp
@@ -42,7 +42,23 @@ int func3() {
 
 int func4() {
   // TODO (&A::foo)(
-  (&A::bar)(
+  (&A::bar)()
 }
 // RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-2):13 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC5 %s
 // CHECK-CC5: OVERLOAD: [#void#](<#A#>, int)
+
+struct C {
+  int member {};
+  void foo(this C& self) {
+    // Should not offer `member` here, since it needs to be 
+    // referenced as `self.member`.
+    mem
+  }
+  void bar(this C& self) {
+    self.mem
+  }
+};
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-8):8 -std=c++23 %s | FileCheck --allow-empty %s
+// CHECK-NOT: COMPLETION: member : [#int#]member
+// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-5):13 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC6 %s
+// CHECK-CC6: COMPLETION: member : [#int#]member

HighCommander4 · 2025-08-16T22:35:19Z

clang/lib/Sema/SemaCodeComplete.cpp

I don't think CurContext is the right DeclContext to check here, as it could be a nested context inside the method. For example:

struct A { int member; void foo(this A &self) { [&]() { // Completion should not offer `member`, but with the current patch it does. mem^ }(); } };

I think you want something like getFunctionLevelDeclContext(/*AllowLambda=*/false).

Also, as mentioned on Discord, we should do this check in CodeCompleteOrdinaryName() and save the result on the ResultBuilder. This is actually important not just for performance, but also to limit the effect of the change to cases where we're in CodeCompleteOrdinaryName(), since ResultBuilder is used for completions in other contexts as well.

Good catch, yeah, I failed to consider this 🤔.

Is storing it with a call to setObjectTypeQualifiers fine? Or is introducing a new get-set pair better?

Is storing it with a call to setObjectTypeQualifiers fine? Or is introducing a new get-set pair better?

I think it would actually make the most sense in an else branch of this if, since getCurrentThisType() should return null in an explicit-object member function (due to this check) (which suggests using a different get/set pair).

How do we ensure that explicit object methods still participate in overload resolution, since the HasObjectTypeQualifiers is false here?

Example, his does not suggest memberFnB

struct A { int member {}; int memberFnA(int a); int memberFnB(this A const& self, float b); void foo(this const A& self) { self.mem^ // suggests only 'member', should suggest `memberFnB` as well self.memberFnA(1); // correctly diagnosed by clangd, but not suggested } };

Should HasObjectTypeQualifiers be true for explicit object methods? I was thinking we can perhaps infer the qualifiers from the this parameter, and reuse setObjectTypeQualifiers to ensure correct overload resolution 🤔?

Good catch, but I would say this is an unrelated bug:

struct A { int foo(this A const&); }; void bar(const A& a) { a.^ // foo not offered }

This scenario involves CodeCompleteMemberReferencExpr, so I don't think it needs to influence what we do in CodeCompleteOrdinaryName. Let's file it as another issue to look at later?

This scenario involves CodeCompleteMemberReferencExpr, so I don't think it needs to influence what we do in CodeCompleteOrdinaryName.

Sounds good!

Let's file it as another issue to look at later?

Looks like it's the first part of #109608.

HighCommander4 · 2025-08-16T22:44:25Z

clang/lib/Sema/SemaCodeComplete.cpp

The check should apply to methods as well as fields:

struct A { int method(); void foo(this A &self) { // Should not offer `method` me^ } };

MythreyaK · 2025-08-17T05:58:02Z

clang/lib/Sema/SemaCodeComplete.cpp

Happy to rename this, wasn't sure of a good name, especially since filters use the IsXxx format.

HighCommander4

Could you add a test case for the nested-scope scenario discussed in this comment please?

(Feel free to pick just one of the libSema and clangd test suites for that one.)

Otherwise looks good!

HighCommander4 · 2025-08-19T03:49:25Z

Also, could you file a follow-up issue about the analogous bug for static functions that we discovered? (You don't necessarily need to work on it, I'd just like to make sure it's on file before we forget about it.)

MythreyaK · 2025-08-19T06:24:56Z

Does this seem okay?

Could you add a test case for the nested-scope scenario discussed in #153760 (comment) please?

Thanks for the heads up! Should be in now.

Also, could you file a follow-up issue about the analogous bug for static functions that we discovered?

Created #154272.

HighCommander4

Thanks!

MythreyaK changed the title ~~[clang] Skip unqualified members in explicit member functions~~ [clang] Skip suggesting unqualified members in explicit-object member functions Aug 15, 2025

MythreyaK force-pushed the mythreyak/deduced-this-member-access branch from 690ca3d to 0af9408 Compare August 15, 2025 08:57

MythreyaK commented Aug 15, 2025

View reviewed changes

MythreyaK force-pushed the mythreyak/deduced-this-member-access branch from 0af9408 to 4953cfb Compare August 15, 2025 09:05

MythreyaK marked this pull request as ready for review August 15, 2025 09:09

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 15, 2025

HighCommander4 reviewed Aug 16, 2025

View reviewed changes

MythreyaK force-pushed the mythreyak/deduced-this-member-access branch 2 times, most recently from 2a3b491 to 9b52103 Compare August 17, 2025 05:31

MythreyaK commented Aug 17, 2025

View reviewed changes

MythreyaK added 3 commits August 17, 2025 17:06

[clang] Skip unqualified members in explicit-member functions

b279e4b

Add codecomplete lit test

650760b

code review

d8cee91

MythreyaK force-pushed the mythreyak/deduced-this-member-access branch from 9b52103 to d8cee91 Compare August 18, 2025 00:06

HighCommander4 requested changes Aug 19, 2025

View reviewed changes

code review

6447c44

HighCommander4 approved these changes Aug 20, 2025

View reviewed changes

HighCommander4 merged commit d0cde32 into llvm:main Aug 20, 2025
9 checks passed

MythreyaK deleted the mythreyak/deduced-this-member-access branch August 20, 2025 06:03

[clang] Skip suggesting unqualified members in explicit-object member functions #153760

[clang] Skip suggesting unqualified members in explicit-object member functions #153760

Uh oh!

Conversation

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

Uh oh!

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

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

Uh oh!

Uh oh!

llvmbot commented Aug 15, 2025

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!

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

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!

HighCommander4 commented Aug 19, 2025

Uh oh!

MythreyaK commented Aug 19, 2025

Uh oh!

HighCommander4 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

MythreyaK commented Aug 15, 2025 •

edited

Loading

MythreyaK Aug 15, 2025 •

edited

Loading

llvmbot commented Aug 15, 2025 •

edited

Loading

MythreyaK Aug 17, 2025 •

edited

Loading