Skip to content

[clang][CodeComplete] skip explicit obj param in SignatureHelp #146649

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.

Sign up for GitHub

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

Merged
HighCommander4 merged 5 commits into from
Jul 31, 2025
+136 −15
Merged

[clang][CodeComplete] skip explicit obj param in SignatureHelp #146649

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
816845b
[clang][CodeComplete] skip explicit obj param in SignatureHelp
MythreyaK Jul 2, 2025
caf4482
Update tests from code review
MythreyaK Jul 3, 2025
e08005e
rebase updates
MythreyaK Jul 29, 2025
203317b
code review
MythreyaK Jul 30, 2025
d719247
code review
MythreyaK Jul 31, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
93 changes: 86 additions & 7 deletions clang-tools-extra/clangd/unittests/CodeCompleteTests.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3267,6 +3267,56 @@ TEST(SignatureHelpTest, VariadicType) {
}
}

TEST(SignatureHelpTest, SkipExplicitObjectParameter) {
Annotations Code(R"cpp(
struct A {
void foo(this auto&& self, int arg);
void bar(this A self, int arg);
};
int main() {
A a {};
a.foo($c1^);
(&A::bar)($c2^);
(&A::foo)($c3^);
}
)cpp");

auto TU = TestTU::withCode(Code.code());
TU.ExtraArgs = {"-std=c++23"};

MockFS FS;
auto Inputs = TU.inputs(FS);

auto Preamble = TU.preamble();
ASSERT_TRUE(Preamble);

{
const auto Result = signatureHelp(testPath(TU.Filename), Code.point("c1"),
*Preamble, Inputs, MarkupKind::PlainText);

EXPECT_EQ(1U, Result.signatures.size());

EXPECT_THAT(Result.signatures[0], AllOf(sig("foo([[int arg]]) -> void")));
}
{
const auto Result = signatureHelp(testPath(TU.Filename), Code.point("c2"),
*Preamble, Inputs, MarkupKind::PlainText);

EXPECT_EQ(1U, Result.signatures.size());

EXPECT_THAT(Result.signatures[0], AllOf(sig("([[A]], [[int]]) -> void")));
}
{
// TODO: llvm/llvm-project/146649
const auto Result = signatureHelp(testPath(TU.Filename), Code.point("c3"),
*Preamble, Inputs, MarkupKind::PlainText);
// TODO: We expect 1 signature here, with this signature
EXPECT_EQ(0U, Result.signatures.size());
// EXPECT_THAT(Result.signatures[0], AllOf(sig("([[auto&&]], [[int]]) ->
// void")));
}
}

TEST(CompletionTest, IncludedCompletionKinds) {
Annotations Test(R"cpp(#include "^)cpp");
auto TU = TestTU::withCode(Test.code());
Expand Down Expand Up @@ -4369,14 +4419,24 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {
Annotations Code(R"cpp(
struct A {
void foo(this auto&& self, int arg);
void bar(this A self, int arg);
};

int main() {
A a {};
a.^
a.$c1^;
(&A::fo$c2^;
(&A::ba$c3^;
}
)cpp");

// TODO: llvm/llvm-project/146649
// This is incorrect behavior. Correct Result should be a variant of,
// c2: signature = (auto&& self, int arg)
// snippet = (${1: auto&& self}, ${2: int arg})
// c3: signature = (A self, int arg)
// snippet = (${1: A self}, ${2: int arg})

auto TU = TestTU::withCode(Code.code());
TU.ExtraArgs = {"-std=c++23"};

Expand All @@ -4387,12 +4447,31 @@ TEST(CompletionTest, SkipExplicitObjectParameter) {

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})"))));
{
auto Result = codeComplete(testPath(TU.Filename), Code.point("c1"),
Preamble.get(), Inputs, Opts);

EXPECT_THAT(Result.Completions,
UnorderedElementsAre(AllOf(named("foo"), signature("(int arg)"),
snippetSuffix("(${1:int arg})")),
AllOf(named("bar"), signature("(int arg)"),
snippetSuffix("(${1:int arg})"))));
}
{
auto Result = codeComplete(testPath(TU.Filename), Code.point("c2"),
Preamble.get(), Inputs, Opts);
EXPECT_THAT(
Result.Completions,
ElementsAre(AllOf(named("foo"), signature("<class self:auto>(int arg)"),
snippetSuffix("<${1:class self:auto}>"))));
Copy link
Collaborator

@HighCommander4 HighCommander4 Jul 30, 2025

Choose a reason for hiding this comment

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

Multiple things about this seem to be wrong:

  1. We should not get an explicit template parameter list in the snippet, because the template parameter can be deduced from the first argument. (We're expecting the user to write (&A::foo)(a, 42); most of the time, not (&A::foo<A>)(a, 42);).
  2. The snippet suffix is missing function parameters altogether.
  3. The signature should have two function parameters, not one.

It's fine to assert the current (buggy) behaviour, but please add a comment describing why it's wrong, and what we should get instead.

}
{
auto Result = codeComplete(testPath(TU.Filename), Code.point("c3"),
Preamble.get(), Inputs, Opts);
EXPECT_THAT(Result.Completions,
ElementsAre(AllOf(named("bar"), signature("(int arg)"),
snippetSuffix(""))));
}
}
} // namespace
} // namespace clangd
Expand Down
8 changes: 8 additions & 0 deletions clang/lib/Sema/SemaCodeComplete.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4034,6 +4034,14 @@ static void AddOverloadParameterChunks(
return;
}

// 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 (Function && FirstParameter &&
Function->getParamDecl(P)->isExplicitObjectParameter()) {
continue;
}

if (FirstParameter)
FirstParameter = false;
else
Expand Down
50 changes: 42 additions & 8 deletions clang/test/CodeCompletion/skip-explicit-object-parameter.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,48 @@
struct A {
void foo(this A self, int arg);
void foo(this auto&& self, int arg);
void bar(this A self, int arg);
};

int main() {
int func1() {
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()
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-2):5 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC1 %s
// CHECK-CC1: COMPLETION: A : A::
// CHECK-NEXT-CC1: COMPLETION: bar : [#void#]bar(<#int arg#>)
// CHECK-NEXT-CC1: COMPLETION: foo : [#void#]foo(<#int arg#>)
// CHECK-NEXT-CC1: COMPLETION: operator= : [#A &#]operator=(<#const A &#>)
// CHECK-NEXT-CC1: COMPLETION: operator= : [#A &#]operator=(<#A &&#>)
// CHECK-NEXT-CC1: COMPLETION: ~A : [#void#]~A()

struct B {
template <typename T>
void foo(this T&& self, int arg);
};

int func2() {
B b {};
b.foo();
}
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-2):9 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC2 %s
// CHECK-CC2: OVERLOAD: [#void#]foo(int arg)
Copy link
Contributor Author

@MythreyaK MythreyaK Jul 3, 2025

Choose a reason for hiding this comment

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

My current understanding is that this line checks that the given overload exists, but not that other overloads do not. Do I need to ensure that this is the only overload?

Copy link
Collaborator

@HighCommander4 HighCommander4 Jul 29, 2025

Choose a reason for hiding this comment

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

It may be possible with CHECK-NOT, but to be honest I wouldn't bother trying to express any non-trivial logic in this test suite. The clangd unit test has coverage for this (it will start failing if there are multiple overloads due to our use of ElementsAre().)


// TODO: llvm/llvm-project/146649
// This is incorrect behavior. Correct Result should be a variant of,
// CC3: should be something like [#void#]foo(<#A self#>, <#int arg#>)
// CC4: should be something like [#void#]bar(<#A self#>, <#int arg#>)
int func3() {
(&A::foo)
(&A::bar)
}
// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-3):10 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC3 %s
// CHECK-CC3: COMPLETION: foo : [#void#]foo<<#class self:auto#>>(<#int arg#>)
Copy link
Collaborator

@HighCommander4 HighCommander4 Jul 30, 2025

Choose a reason for hiding this comment

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

(As discussed in the unit test, this signature is not what we want and we should add a comment saying so.)

// RUN: %clang_cc1 -fsyntax-only -code-completion-at=%s:%(line-4):10 -std=c++23 %s | FileCheck -check-prefix=CHECK-CC4 %s
// CHECK-CC4: COMPLETION: bar : [#void#]bar(<#int arg#>)

int func4() {
// TODO (&A::foo)(
(&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)