Skip to content

Commit

Permalink
[clangd] Carefully handle PseudoObjectExprs for inlay hints (#71366)
Browse files Browse the repository at this point in the history
Not all subexpressions for a PseudoObjectExpr are interesting, and they
probably mess up inlay hints.

Fixes clangd/clangd#1813.
  • Loading branch information
zyn0217 committed Dec 3, 2023
1 parent 76cd035 commit 0fc69b1
Show file tree
Hide file tree
Showing 2 changed files with 59 additions and 1 deletion.
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

0 comments on commit 0fc69b1

Please sign in to comment.