[clang] skip explicit obj param in code complete

[MythreyaK]

Skips the first explicit object parameter when generating autocomplete suggestion string

Fixes clangd/clangd#2339?

[HighCommander4]

Thanks! This is a nice and simple fix.

• It fixes the main issue as originally reported.
• It fixes the additional issue discussed in this comment, taking the second approach (rendering the signature as f(int arg)), which I think is perfectly fine as that's what falls out naturally from this implementation.
• It does not fix #2284, which is fine, it was just speculation on my part that #2339 and #2284 may have the same cause. (That said, if you're interested, #2284 would make a good follow-up issue to investigate next.)

The clangd test looks great, thanks for writing it! That said, based on my experience contributing changes to SemaCodeComplete.cpp, people like to also ask for a test to be added to libSema's own code completion test suite. These tests work a bit differently: they're found at clang/test/CodeCompletion (see for example this file), and they operate by running clang with the special -code-complete-at flag which invokes code completion at a point in the input file and prints the results in some format.

So for example, given test.cpp containing:

struct A
{
    void f(this A self, int arg);
};

int main() {
  A a;
  a.f
}

The command clang -cc1 -fsyntax-only -code-completion-at=test.cpp:8:5 -std=c++23 test.cpp would previously print:

COMPLETION: A : A::
COMPLETION: f : [#void#]f(<#A self#>, <#int arg#>)
COMPLETION: operator= : [#A &#]operator=(<#const A &#>)
COMPLETION: operator= : [#A &#]operator=(<#A &&#>)
COMPLETION: ~A : [#void#]~A()

and with this patch it prints:

COMPLETION: A : A::
COMPLETION: f : [#void#]f(<#int arg#>)
COMPLETION: operator= : [#A &#]operator=(<#const A &#>)
COMPLETION: operator= : [#A &#]operator=(<#A &&#>)
COMPLETION: ~A : [#void#]~A()

allowing us to write a test that asserts that the output is the latter.

Would you mind writing a test of this form as well?

[HighCommander4]

For good measure, could you check both signature() and snippetSuffix()?

[HighCommander4]

[...] people like to also ask for a test to be added to libSema's own code completion test suite. These tests work a bit differently: they're found at clang/test/CodeCompletion (see for example this file), and they operate by running clang with the special -code-complete-at flag which invokes code completion at a point in the input file and prints the results in some format.

(In case this test suite seems like dark magic even with that explanation, which is how it initially seemed to me, see https://llvm.org/docs/CommandGuide/lit.html and https://llvm.org/docs/CommandGuide/FileCheck.html for more details of the machinery behind it.)

[MythreyaK]

Thank you for patiently giving me pointers and links! Definitely came in handy!

I just noticed that I didn't follow a pattern with the test naming.

• Something along the lines of "ignore/skip self parameter in deducing this" is more user-friendly
• "ignore/skip explicit object parameter" is probably consistent with the C++ feature

Is SkipExplicitObjectParameter (for the test) and skip-explicit-object-parameter.cpp (for the file) acceptable? Open to suggestions for more descriptive names! (I'm bad with names :D)

[zwuis]

What about "ExplicitObjectMemberFunction"? It seems that this part of feature has not been tested.

[MythreyaK]

What about "ExplicitObjectMemberFunction"? It seems that this part of feature has not been tested.

@zwuis yeah. This addresses only the function arg in the inserted snippet, not the member functions.

Originally posted by @HighCommander4 in #109608,

See also clangd/clangd#2339, another report that boils down to the second of the two problems in the issue description.

I'm still exploring the code for that, so maybe I'll create a separate PR for it if and when I figure it out.

[HighCommander4]

The new test + other updates look great, thanks!

I agree with using the terminology "explicit object parameter", as this is what appears in the clang API (e.g. ParmVarDecl::isExplicitObjectParameter()).

@MythreyaK I'll hold off on merging this until I get a confirmation from you (since the patch is still currently in the "Draft" state), but from my point of view this is good to go.

[llvmbot]

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

@llvm/pr-subscribers-clangd

Author: Mythreya (MythreyaK)

Changes

Skips the first explicit object parameter when generating autocomplete suggestion string

Fixes clangd/clangd#2339?

---

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

3 Files Affected:

• (modified) clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp (+30)
• (modified) clang/lib/Sema/SemaCodeComplete.cpp (+7)
• (added) clang/test/CodeCompletion/skip-explicit-object-parameter.cpp (+14)

diff --git a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
index 311f0d98904ad..b7c64c7a06745 100644
--- a/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
+++ b/clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
@@ -4363,6 +4363,36 @@ TEST(CompletionTest, PreambleFromDifferentTarget) {
   EXPECT_THAT(Result.Completions, Not(testing::IsEmpty()));
   EXPECT_THAT(Signatures.signatures, Not(testing::IsEmpty()));
 }
+
+TEST(CompletionTest, SkipExplicitObjectParameter) {
+  Annotations Code(R"cpp(
+    struct A {
+      void foo(this auto&& self, int arg); 
+    };
+
+    int main() {
+      A a {};
+      a.^
+    }
+  )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(),
+                             Preamble.get(), Inputs, Opts);
+
+  EXPECT_THAT(Result.Completions,
+              ElementsAre(AllOf(named("foo"), signature("(int arg)"),
+                                snippetSuffix("(${1:int arg})"))));
+}
 } // namespace
 } // namespace clangd
 } // namespace clang
diff --git a/clang/lib/Sema/SemaCodeComplete.cpp b/clang/lib/Sema/SemaCodeComplete.cpp
index 78d4586e45978..b5d4a94da83df 100644
--- a/clang/lib/Sema/SemaCodeComplete.cpp
+++ b/clang/lib/Sema/SemaCodeComplete.cpp
@@ -3260,6 +3260,13 @@ static void AddFunctionParameterChunks(Preprocessor &PP,
       break;
     }
 
+    // C++23 introduces an explicit object parameter, a.k.a. "deducing this"
+    // Skip it for autocomplete and treat the next parameter as the first
+    // parameter
+    if (FirstParameter && Param->isExplicitObjectParameter()) {
+      continue;
+    }
+
     if (FirstParameter)
       FirstParameter = false;
     else
diff --git a/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp b/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
new file mode 100644
index 0000000000000..55c16bb126fee
--- /dev/null
+++ b/clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
@@ -0,0 +1,14 @@
+struct A {
+  void foo(this A self, int arg);
+};
+
+int main() {
+  A a {};
+  a.
+}
+// RUN: %clang_cc1 -cc1 -fsyntax-only -code-completion-at=%s:%(line-2):5 -std=c++23 %s | FileCheck %s
+// CHECK: COMPLETION: A : A::
+// CHECK-NEXT: COMPLETION: foo : [#void#]foo(<#int arg#>)
+// CHECK-NEXT: COMPLETION: operator= : [#A &#]operator=(<#const A &#>)
+// CHECK-NEXT: COMPLETION: operator= : [#A &#]operator=(<#A &&#>)
+// CHECK-NEXT: COMPLETION: ~A : [#void#]~A()

[MythreyaK]

this is good to go

Thanks for the confirmation! !