Skip to content

Commit 0fc69b1

Browse files
authored
[clangd] Carefully handle PseudoObjectExprs for inlay hints (#71366)
Not all subexpressions for a PseudoObjectExpr are interesting, and they probably mess up inlay hints. Fixes clangd/clangd#1813.
1 parent 76cd035 commit 0fc69b1

File tree

2 files changed

+59
-1
lines changed

2 files changed

+59
-1
lines changed

clang-tools-extra/clangd/InlayHints.cpp

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,31 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
589589
return true;
590590
}
591591

592+
// Carefully recurse into PseudoObjectExprs, which typically incorporate
593+
// a syntactic expression and several semantic expressions.
594+
bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
595+
Expr *SyntacticExpr = E->getSyntacticForm();
596+
if (isa<CallExpr>(SyntacticExpr))
597+
// Since the counterpart semantics usually get the identical source
598+
// locations as the syntactic one, visiting those would end up presenting
599+
// confusing hints e.g., __builtin_dump_struct.
600+
// Thus, only traverse the syntactic forms if this is written as a
601+
// CallExpr. This leaves the door open in case the arguments in the
602+
// syntactic form could possibly get parameter names.
603+
return RecursiveASTVisitor<InlayHintVisitor>::TraverseStmt(SyntacticExpr);
604+
// We don't want the hints for some of the MS property extensions.
605+
// e.g.
606+
// struct S {
607+
// __declspec(property(get=GetX, put=PutX)) int x[];
608+
// void PutX(int y);
609+
// void Work(int y) { x = y; } // Bad: `x = y: y`.
610+
// };
611+
if (isa<BinaryOperator>(SyntacticExpr))
612+
return true;
613+
// FIXME: Handle other forms of a pseudo object expression.
614+
return RecursiveASTVisitor<InlayHintVisitor>::TraversePseudoObjectExpr(E);
615+
}
616+
592617
bool VisitCallExpr(CallExpr *E) {
593618
if (!Cfg.InlayHints.Parameters)
594619
return true;

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

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1709,7 +1709,8 @@ TEST(DesignatorHints, NoCrash) {
17091709
void test() {
17101710
Foo f{A(), $b[[1]]};
17111711
}
1712-
)cpp", ExpectedHint{".b=", "b"});
1712+
)cpp",
1713+
ExpectedHint{".b=", "b"});
17131714
}
17141715

17151716
TEST(InlayHints, RestrictRange) {
@@ -1724,6 +1725,38 @@ TEST(InlayHints, RestrictRange) {
17241725
ElementsAre(labelIs(": int"), labelIs(": char")));
17251726
}
17261727

1728+
TEST(ParameterHints, PseudoObjectExpr) {
1729+
Annotations Code(R"cpp(
1730+
struct S {
1731+
__declspec(property(get=GetX, put=PutX)) int x[];
1732+
int GetX(int y, int z) { return 42 + y; }
1733+
void PutX(int) { }
1734+
1735+
// This is a PseudoObjectExpression whose syntactic form is a binary
1736+
// operator.
1737+
void Work(int y) { x = y; } // Not `x = y: y`.
1738+
};
1739+
1740+
int printf(const char *Format, ...);
1741+
1742+
int main() {
1743+
S s;
1744+
__builtin_dump_struct(&s, printf); // Not `Format: __builtin_dump_struct()`
1745+
printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
1746+
// This builds a PseudoObjectExpr, but here it's useful for showing the
1747+
// arguments from the semantic form.
1748+
return s.x[ $one[[1]] ][ $two[[2]] ]; // `x[y: 1][z: 2]`
1749+
}
1750+
)cpp");
1751+
auto TU = TestTU::withCode(Code.code());
1752+
TU.ExtraArgs.push_back("-fms-extensions");
1753+
auto AST = TU.build();
1754+
EXPECT_THAT(inlayHints(AST, std::nullopt),
1755+
ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, Code),
1756+
HintMatcher(ExpectedHint{"y: ", "one"}, Code),
1757+
HintMatcher(ExpectedHint{"z: ", "two"}, Code)));
1758+
}
1759+
17271760
TEST(ParameterHints, ArgPacksAndConstructors) {
17281761
assertParameterHints(
17291762
R"cpp(

0 commit comments

Comments
 (0)