Skip to content

[clangd] Carefully handle PseudoObjectExprs for inlay hints #71366

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

Merged
merged 7 commits into from
Dec 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions clang-tools-extra/clangd/InlayHints.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -589,6 +589,31 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
return true;
}

// Carefully recurse into PseudoObjectExprs, which typically incorporate
// a syntactic expression and several semantic expressions.
bool TraversePseudoObjectExpr(PseudoObjectExpr *E) {
Expr *SyntacticExpr = E->getSyntacticForm();
if (isa<CallExpr>(SyntacticExpr))
// Since the counterpart semantics usually get the identical source
// locations as the syntactic one, visiting those would end up presenting
// confusing hints e.g., __builtin_dump_struct.
// Thus, only traverse the syntactic forms if this is written as a
// CallExpr. This leaves the door open in case the arguments in the
// syntactic form could possibly get parameter names.
return RecursiveASTVisitor<InlayHintVisitor>::TraverseStmt(SyntacticExpr);
// We don't want the hints for some of the MS property extensions.
// e.g.
// struct S {
// __declspec(property(get=GetX, put=PutX)) int x[];
// void PutX(int y);
// void Work(int y) { x = y; } // Bad: `x = y: y`.
// };
if (isa<BinaryOperator>(SyntacticExpr))
return true;
// FIXME: Handle other forms of a pseudo object expression.
return RecursiveASTVisitor<InlayHintVisitor>::TraversePseudoObjectExpr(E);
}

bool VisitCallExpr(CallExpr *E) {
if (!Cfg.InlayHints.Parameters)
return true;
Expand Down
35 changes: 34 additions & 1 deletion clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -1709,7 +1709,8 @@ TEST(DesignatorHints, NoCrash) {
void test() {
Foo f{A(), $b[[1]]};
}
)cpp", ExpectedHint{".b=", "b"});
)cpp",
ExpectedHint{".b=", "b"});
}

TEST(InlayHints, RestrictRange) {
Expand All @@ -1724,6 +1725,38 @@ TEST(InlayHints, RestrictRange) {
ElementsAre(labelIs(": int"), labelIs(": char")));
}

TEST(ParameterHints, PseudoObjectExpr) {
Annotations Code(R"cpp(
struct S {
__declspec(property(get=GetX, put=PutX)) int x[];
int GetX(int y, int z) { return 42 + y; }
void PutX(int) { }

// This is a PseudoObjectExpression whose syntactic form is a binary
// operator.
void Work(int y) { x = y; } // Not `x = y: y`.
};

int printf(const char *Format, ...);

int main() {
S s;
__builtin_dump_struct(&s, printf); // Not `Format: __builtin_dump_struct()`
printf($Param[["Hello, %d"]], 42); // Normal calls are not affected.
// This builds a PseudoObjectExpr, but here it's useful for showing the
// arguments from the semantic form.
return s.x[ $one[[1]] ][ $two[[2]] ]; // `x[y: 1][z: 2]`
}
)cpp");
auto TU = TestTU::withCode(Code.code());
TU.ExtraArgs.push_back("-fms-extensions");
auto AST = TU.build();
EXPECT_THAT(inlayHints(AST, std::nullopt),
ElementsAre(HintMatcher(ExpectedHint{"Format: ", "Param"}, Code),
HintMatcher(ExpectedHint{"y: ", "one"}, Code),
HintMatcher(ExpectedHint{"z: ", "two"}, Code)));
}

TEST(ParameterHints, ArgPacksAndConstructors) {
assertParameterHints(
R"cpp(
Expand Down