-
Notifications
You must be signed in to change notification settings - Fork 15.4k
[clangd] Add designator hints for parenthesis aggregate initialization #170642
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
base: main
Are you sure you want to change the base?
Conversation
zwuis
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please add tests with classes having base classes? Aggregates are allowed to have base classes since C++17.
https://en.cppreference.com/w/cpp/language/aggregate_initialization.html
| struct S1 { | ||
| int a; | ||
| int b; | ||
| }; | ||
| struct S2 : S1 { | ||
| int c; | ||
| int d; | ||
| }; | ||
| S2 s2 ({$a[[0]], $b[[0]]}, $c[[0]], $d[[0]]); | ||
| )cpp", | ||
| // ExpectedHint{"S1:", "S1"}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add some form of hint here?
S2 s2 ({0, 0}, 0, 0); // S2 s2 ({.a=0, .b=0}, .c=0, .d=0); // current
S2 s2 ({0, 0}, 0, 0); // S2 s2 (S1:{.a=0, .b=0}, .c=0, .d=0);
// Some hint here? -------------^There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@HighCommander4 suggested we add this in a separate PR so that it's easy to revert if that hint is not well received. Will create a separate PR for it.
|
I think I'm seeing a bug. For this snippet, template <typename T>
struct S1 {
int a;
int b;
T* ptr;
};
struct S2 : S1<S2> {
int c;
int d;
S1<S1> mem;
};
S2 s2 ({0, 0}, 0, 0);
S2 s2 ({$a[[0]], $b[[0]]}, $c[[0]], $d[[0]]);source range for mem is |
|
I'm not sure if it is a bug. But it doesn't block this PR because no need to emit designator hints for Update: Oh I see. We need to distinguish between these cases: struct S1 {};
struct S2 : S1 {
int a;
S1 s1;
};
struct S3 { S3() {} };
struct S4 : S3 {
int a;
S3 s3;
};
S2 s2({}, 0);
S4 s4({}, 0, {}); |
|
We can use |
Didn't notice that; Thank you, that fixed it! |
|
@llvm/pr-subscribers-clang-tools-extra Author: Mythreya Kuricheti (MythreyaK) ChangesEnabled hints for struct Point {
int x;
int y;
};
int main() {
Point p1{1, 2}; // brace syntax <-- already present
Point p2(1, 2); // parens syntax <-- added in this commit
}<img width="434" height="413" alt="image" src="https://github.com/user-attachments/assets/710d7168-30f7-494d-8b2c-6413fa4152a7" /> Fixes clangd/clangd#2540 Full diff: https://github.com/llvm/llvm-project/pull/170642.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 23bd02304a4fd..040015cfc2d13 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -699,6 +699,33 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return InstantiatedFunction->getParamDecl(ParamIdx);
}
+ bool VisitCXXParenListInitExpr(CXXParenListInitExpr *E) {
+ if (!Cfg.InlayHints.Designators)
+ return true;
+
+ if (const auto *CXXRecord = E->getType()->getAsCXXRecordDecl()) {
+ const auto &InitExprs = E->getUserSpecifiedInitExprs();
+ size_t InitInx = 0;
+
+ // Inherited members are first, skip them for now.
+ // FIXME: '.base=' or 'base:' hint?
+ for (const auto &_ : CXXRecord->bases()) {
+ InitInx++;
+ }
+
+ // Then the members defined in this record
+ auto RecordFields = CXXRecord->fields().begin();
+
+ for (; InitInx < InitExprs.size();
+ ++InitInx, RecordFields = std::next(RecordFields)) {
+ addDesignatorHint(InitExprs[InitInx]->getSourceRange(),
+ "." + RecordFields->getName().str());
+ }
+ }
+
+ return true;
+ }
+
bool VisitInitListExpr(InitListExpr *Syn) {
// We receive the syntactic form here (shouldVisitImplicitCode() is false).
// This is the one we will ultimately attach designators to.
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index feb4404b3d2bf..d54e36c2f8a3b 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1147,20 +1147,6 @@ TEST(ParameterHints, CopyOrMoveConstructor) {
)cpp");
}
-TEST(ParameterHints, AggregateInit) {
- // FIXME: This is not implemented yet, but it would be a natural
- // extension to show member names as hints here.
- assertParameterHints(R"cpp(
- struct Point {
- int x;
- int y;
- };
- void bar() {
- Point p{41, 42};
- }
- )cpp");
-}
-
TEST(ParameterHints, UserDefinedLiteral) {
// Do not hint call to user-defined literal operator.
assertParameterHints(R"cpp(
@@ -1746,6 +1732,19 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
}
TEST(DesignatorHints, Basic) {
+ assertDesignatorHints(R"cpp(
+ struct Point {
+ int x;
+ int y;
+ };
+ void bar() {
+ Point p{$x[[41]], $y[[42]]};
+ }
+ )cpp",
+ ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"});
+}
+
+TEST(DesignatorHints, BasicArray) {
assertDesignatorHints(R"cpp(
struct S { int x, y, z; };
S s {$x[[1]], $y[[2+2]]};
@@ -1822,6 +1821,63 @@ TEST(DesignatorHints, NoCrash) {
ExpectedHint{".b=", "b"});
}
+TEST(DesignatorHints, BasicParenInit) {
+ assertDesignatorHints(R"cpp(
+ struct S {
+ int x;
+ int y;
+ int z;
+ };
+ S s ($x[[1]], $y[[2+2]], $z[[4]]);
+ )cpp",
+ ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"},
+ ExpectedHint{".z=", "z"});
+}
+
+TEST(DesignatorHints, BasicParenInitDerived) {
+ assertDesignatorHints(R"cpp(
+ struct S1 {
+ int a;
+ int b;
+ };
+
+ struct S2 : S1 {
+ int c;
+ int d;
+ };
+ S2 s2 ({$a[[0]], $b[[0]]}, $c[[0]], $d[[0]]);
+ )cpp",
+ // ExpectedHint{"S1:", "S1"},
+ ExpectedHint{".a=", "a"}, ExpectedHint{".b=", "b"},
+ ExpectedHint{".c=", "c"}, ExpectedHint{".d=", "d"});
+}
+
+TEST(DesignatorHints, BasicParenInitTemplate) {
+ assertDesignatorHints(R"cpp(
+ template <typename T>
+ struct S1 {
+ int a;
+ int b;
+ T* ptr;
+ };
+
+ struct S2 : S1<S2> {
+ int c;
+ int d;
+ S1<int> mem;
+ };
+
+ int main() {
+ S2 sa ({$a1[[0]], $b1[[0]]}, $c[[0]], $d[[0]], $mem[[S1<int>($a2[[1]], $b2[[2]], $ptr[[nullptr]])]]);
+ }
+ )cpp",
+ ExpectedHint{".a=", "a1"}, ExpectedHint{".b=", "b1"},
+ ExpectedHint{".c=", "c"}, ExpectedHint{".d=", "d"},
+ ExpectedHint{".mem=", "mem"}, ExpectedHint{".a=", "a2"},
+ ExpectedHint{".b=", "b2"},
+ ExpectedHint{".ptr=", "ptr"});
+}
+
TEST(InlayHints, RestrictRange) {
Annotations Code(R"cpp(
auto a = false;
|
|
@llvm/pr-subscribers-clangd Author: Mythreya Kuricheti (MythreyaK) ChangesEnabled hints for struct Point {
int x;
int y;
};
int main() {
Point p1{1, 2}; // brace syntax <-- already present
Point p2(1, 2); // parens syntax <-- added in this commit
}<img width="434" height="413" alt="image" src="https://github.com/user-attachments/assets/710d7168-30f7-494d-8b2c-6413fa4152a7" /> Fixes clangd/clangd#2540 Full diff: https://github.com/llvm/llvm-project/pull/170642.diff 2 Files Affected:
diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp
index 23bd02304a4fd..040015cfc2d13 100644
--- a/clang-tools-extra/clangd/InlayHints.cpp
+++ b/clang-tools-extra/clangd/InlayHints.cpp
@@ -699,6 +699,33 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return InstantiatedFunction->getParamDecl(ParamIdx);
}
+ bool VisitCXXParenListInitExpr(CXXParenListInitExpr *E) {
+ if (!Cfg.InlayHints.Designators)
+ return true;
+
+ if (const auto *CXXRecord = E->getType()->getAsCXXRecordDecl()) {
+ const auto &InitExprs = E->getUserSpecifiedInitExprs();
+ size_t InitInx = 0;
+
+ // Inherited members are first, skip them for now.
+ // FIXME: '.base=' or 'base:' hint?
+ for (const auto &_ : CXXRecord->bases()) {
+ InitInx++;
+ }
+
+ // Then the members defined in this record
+ auto RecordFields = CXXRecord->fields().begin();
+
+ for (; InitInx < InitExprs.size();
+ ++InitInx, RecordFields = std::next(RecordFields)) {
+ addDesignatorHint(InitExprs[InitInx]->getSourceRange(),
+ "." + RecordFields->getName().str());
+ }
+ }
+
+ return true;
+ }
+
bool VisitInitListExpr(InitListExpr *Syn) {
// We receive the syntactic form here (shouldVisitImplicitCode() is false).
// This is the one we will ultimately attach designators to.
diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
index feb4404b3d2bf..d54e36c2f8a3b 100644
--- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -1147,20 +1147,6 @@ TEST(ParameterHints, CopyOrMoveConstructor) {
)cpp");
}
-TEST(ParameterHints, AggregateInit) {
- // FIXME: This is not implemented yet, but it would be a natural
- // extension to show member names as hints here.
- assertParameterHints(R"cpp(
- struct Point {
- int x;
- int y;
- };
- void bar() {
- Point p{41, 42};
- }
- )cpp");
-}
-
TEST(ParameterHints, UserDefinedLiteral) {
// Do not hint call to user-defined literal operator.
assertParameterHints(R"cpp(
@@ -1746,6 +1732,19 @@ TEST(TypeHints, SubstTemplateParameterAliases) {
}
TEST(DesignatorHints, Basic) {
+ assertDesignatorHints(R"cpp(
+ struct Point {
+ int x;
+ int y;
+ };
+ void bar() {
+ Point p{$x[[41]], $y[[42]]};
+ }
+ )cpp",
+ ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"});
+}
+
+TEST(DesignatorHints, BasicArray) {
assertDesignatorHints(R"cpp(
struct S { int x, y, z; };
S s {$x[[1]], $y[[2+2]]};
@@ -1822,6 +1821,63 @@ TEST(DesignatorHints, NoCrash) {
ExpectedHint{".b=", "b"});
}
+TEST(DesignatorHints, BasicParenInit) {
+ assertDesignatorHints(R"cpp(
+ struct S {
+ int x;
+ int y;
+ int z;
+ };
+ S s ($x[[1]], $y[[2+2]], $z[[4]]);
+ )cpp",
+ ExpectedHint{".x=", "x"}, ExpectedHint{".y=", "y"},
+ ExpectedHint{".z=", "z"});
+}
+
+TEST(DesignatorHints, BasicParenInitDerived) {
+ assertDesignatorHints(R"cpp(
+ struct S1 {
+ int a;
+ int b;
+ };
+
+ struct S2 : S1 {
+ int c;
+ int d;
+ };
+ S2 s2 ({$a[[0]], $b[[0]]}, $c[[0]], $d[[0]]);
+ )cpp",
+ // ExpectedHint{"S1:", "S1"},
+ ExpectedHint{".a=", "a"}, ExpectedHint{".b=", "b"},
+ ExpectedHint{".c=", "c"}, ExpectedHint{".d=", "d"});
+}
+
+TEST(DesignatorHints, BasicParenInitTemplate) {
+ assertDesignatorHints(R"cpp(
+ template <typename T>
+ struct S1 {
+ int a;
+ int b;
+ T* ptr;
+ };
+
+ struct S2 : S1<S2> {
+ int c;
+ int d;
+ S1<int> mem;
+ };
+
+ int main() {
+ S2 sa ({$a1[[0]], $b1[[0]]}, $c[[0]], $d[[0]], $mem[[S1<int>($a2[[1]], $b2[[2]], $ptr[[nullptr]])]]);
+ }
+ )cpp",
+ ExpectedHint{".a=", "a1"}, ExpectedHint{".b=", "b1"},
+ ExpectedHint{".c=", "c"}, ExpectedHint{".d=", "d"},
+ ExpectedHint{".mem=", "mem"}, ExpectedHint{".a=", "a2"},
+ ExpectedHint{".b=", "b2"},
+ ExpectedHint{".ptr=", "ptr"});
+}
+
TEST(InlayHints, RestrictRange) {
Annotations Code(R"cpp(
auto a = false;
|
Co-authored-by: zwuis <zwuis@outlook.com>

Enabled hints for
CXXParenListInitExpr,Fixes clangd/clangd#2540