https://github.com/zyn0217 updated https://github.com/llvm/llvm-project/pull/71366
>From 808c141c34218dd542b00149216adc061567dd31 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 6 Nov 2023 16:50:02 +0800 Subject: [PATCH 1/6] [clangd] Don't show inlay hints for PseudoObjectExprs in C++ This closes https://github.com/clangd/clangd/issues/1813. PseudoObjectExprs in C++ are currently not very interesting but probably mess up inlay hints. --- clang-tools-extra/clangd/InlayHints.cpp | 13 ++++++++++ .../clangd/unittests/InlayHintTests.cpp | 24 +++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 3da047f95421385..867c70e5dbc80c6 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -589,6 +589,19 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { return true; } + bool dataTraverseStmtPre(Stmt *S) { + // Do not show inlay hints for PseudoObjectExprs. They're never + // genuine user codes in C++. + // + // For example, __builtin_dump_struct would expand to a PseudoObjectExpr + // that includes a couple of calls to a printf function. Printing parameter + // names for that anyway would end up with duplicate parameter names (which, + // however, got de-duplicated after visiting) for the printf function. + if (AST.getLangOpts().CPlusPlus && isa<PseudoObjectExpr>(S)) + return false; + return true; + } + bool VisitCallExpr(CallExpr *E) { if (!Cfg.InlayHints.Parameters) return true; diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 20c1cdd985dbc01..2b6c27b17b537a9 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1724,6 +1724,30 @@ 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 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. + return s.x[1][2]; // Not `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))); +} + TEST(ParameterHints, ArgPacksAndConstructors) { assertParameterHints( R"cpp( >From f15e35f67e23e742536ca2fe7229f5b6cba8f6b8 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 6 Nov 2023 17:32:50 +0800 Subject: [PATCH 2/6] Don't make the patch C++-specific --- clang-tools-extra/clangd/InlayHints.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 867c70e5dbc80c6..1b54b570c1c5d4f 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -591,13 +591,13 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { bool dataTraverseStmtPre(Stmt *S) { // Do not show inlay hints for PseudoObjectExprs. They're never - // genuine user codes in C++. + // genuine user codes. // // For example, __builtin_dump_struct would expand to a PseudoObjectExpr // that includes a couple of calls to a printf function. Printing parameter // names for that anyway would end up with duplicate parameter names (which, // however, got de-duplicated after visiting) for the printf function. - if (AST.getLangOpts().CPlusPlus && isa<PseudoObjectExpr>(S)) + if (isa<PseudoObjectExpr>(S)) return false; return true; } >From 8813a4f11254f89feb72e435888d896911a5dc78 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Tue, 7 Nov 2023 14:22:07 +0800 Subject: [PATCH 3/6] Only apply the restriction to __builtin_dump_struct --- clang-tools-extra/clangd/InlayHints.cpp | 18 +++++++++--------- .../clangd/unittests/InlayHintTests.cpp | 9 ++++++--- 2 files changed, 15 insertions(+), 12 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 1b54b570c1c5d4f..9897d1c1e6da209 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -590,15 +590,15 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { } bool dataTraverseStmtPre(Stmt *S) { - // Do not show inlay hints for PseudoObjectExprs. They're never - // genuine user codes. - // - // For example, __builtin_dump_struct would expand to a PseudoObjectExpr - // that includes a couple of calls to a printf function. Printing parameter - // names for that anyway would end up with duplicate parameter names (which, - // however, got de-duplicated after visiting) for the printf function. - if (isa<PseudoObjectExpr>(S)) - return false; + // Do not show inlay hints for the __builtin_dump_struct, which would expand + // to a PseudoObjectExpr that includes a couple of calls to a printf + // function. Printing parameter names for that anyway would end up with + // duplicate parameter names (which, however, got de-duplicated after + // visiting) for the printf function. + if (auto *POE = dyn_cast<PseudoObjectExpr>(S)) + if (auto *CE = dyn_cast<CallExpr>(POE->getSyntacticForm()); + CE && CE->getBuiltinCallee() == Builtin::BI__builtin_dump_struct) + return false; return true; } diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 2b6c27b17b537a9..47af261fad850e8 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1729,7 +1729,7 @@ TEST(ParameterHints, PseudoObjectExpr) { struct S { __declspec(property(get=GetX, put=PutX)) int x[]; int GetX(int y, int z) { return 42 + y; } - void PutX(int y) { x = y; } // Not `x = y: y` + void PutX(int y) { x = $one[[y]]; } // FIXME: Undesired `x = y: y` for this ill-formed expression. }; int printf(const char *Format, ...); @@ -1738,14 +1738,17 @@ TEST(ParameterHints, PseudoObjectExpr) { S s; __builtin_dump_struct(&s, printf); // Not `Format: __builtin_dump_struct()` printf($Param[["Hello, %d"]], 42); // Normal calls are not affected. - return s.x[1][2]; // Not `x[y: 1][z: 2]` + return s.x[ $two[[1]] ][ $three[[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))); + ElementsAre(HintMatcher(ExpectedHint{"y: ", "one"}, Code), + HintMatcher(ExpectedHint{"Format: ", "Param"}, Code), + HintMatcher(ExpectedHint{"y: ", "two"}, Code), + HintMatcher(ExpectedHint{"z: ", "three"}, Code))); } TEST(ParameterHints, ArgPacksAndConstructors) { >From c88aad6d290cb8e2732f7fae71cb2ea2ecf0abda Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Mon, 20 Nov 2023 00:14:17 +0800 Subject: [PATCH 4/6] Override TraversePseudoObjectExpr --- clang-tools-extra/clangd/InlayHints.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/clang-tools-extra/clangd/InlayHints.cpp b/clang-tools-extra/clangd/InlayHints.cpp index 9897d1c1e6da209..b460baffee8802d 100644 --- a/clang-tools-extra/clangd/InlayHints.cpp +++ b/clang-tools-extra/clangd/InlayHints.cpp @@ -589,17 +589,22 @@ class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> { return true; } - bool dataTraverseStmtPre(Stmt *S) { - // Do not show inlay hints for the __builtin_dump_struct, which would expand - // to a PseudoObjectExpr that includes a couple of calls to a printf - // function. Printing parameter names for that anyway would end up with - // duplicate parameter names (which, however, got de-duplicated after + bool TraversePseudoObjectExpr(PseudoObjectExpr *E) { + // Do not show inlay hints for the __builtin_dump_struct, which would + // expand to a PseudoObjectExpr that includes a couple of calls to a + // printf function. Printing parameter names for that anyway would end up + // with duplicate parameter names (which, however, got de-duplicated after // visiting) for the printf function. - if (auto *POE = dyn_cast<PseudoObjectExpr>(S)) - if (auto *CE = dyn_cast<CallExpr>(POE->getSyntacticForm()); - CE && CE->getBuiltinCallee() == Builtin::BI__builtin_dump_struct) - return false; - return true; + if (auto *CE = dyn_cast<CallExpr>(E->getSyntacticForm()); + CE && CE->getBuiltinCallee() == Builtin::BI__builtin_dump_struct) + // Only traverse the syntactic forms. This leaves the door open in case + // the arguments in the syntactic form for __builtin_dump_struct could + // possibly get parameter names. + return RecursiveASTVisitor<InlayHintVisitor>::TraverseStmt( + E->getSyntacticForm()); + // FIXME: Shall we ignore semantic forms for other pseudo object + // expressions? + return RecursiveASTVisitor<InlayHintVisitor>::TraversePseudoObjectExpr(E); } bool VisitCallExpr(CallExpr *E) { >From ad5036adf86b4ea99cd3c3c231119ff48792ff6b Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Tue, 21 Nov 2023 00:19:12 +0800 Subject: [PATCH 5/6] Address some comments on the test. --- clang-tools-extra/clangd/unittests/InlayHintTests.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index 47af261fad850e8..f9f7fde36c14897 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1729,7 +1729,8 @@ TEST(ParameterHints, PseudoObjectExpr) { struct S { __declspec(property(get=GetX, put=PutX)) int x[]; int GetX(int y, int z) { return 42 + y; } - void PutX(int y) { x = $one[[y]]; } // FIXME: Undesired `x = y: y` for this ill-formed expression. + // FIXME: Undesired hint `x = y: y`. + void PutX(int y) { x = $one[[y]]; } }; int printf(const char *Format, ...); @@ -1738,6 +1739,8 @@ TEST(ParameterHints, PseudoObjectExpr) { 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[ $two[[1]] ][ $three[[2]] ]; // `x[y: 1][z: 2]` } )cpp"); >From e2febc5fd5b007b26037e9feaf3de9f751e1cf28 Mon Sep 17 00:00:00 2001 From: Younan Zhang <zyn7...@gmail.com> Date: Tue, 21 Nov 2023 00:23:59 +0800 Subject: [PATCH 6/6] Append a comment --- clang-tools-extra/clangd/unittests/InlayHintTests.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp index f9f7fde36c14897..db1d22730da5168 100644 --- a/clang-tools-extra/clangd/unittests/InlayHintTests.cpp +++ b/clang-tools-extra/clangd/unittests/InlayHintTests.cpp @@ -1729,7 +1729,7 @@ TEST(ParameterHints, PseudoObjectExpr) { struct S { __declspec(property(get=GetX, put=PutX)) int x[]; int GetX(int y, int z) { return 42 + y; } - // FIXME: Undesired hint `x = y: y`. + // FIXME: Undesired hint `x = y: y`. This builds a PseudoObjectExpr too. void PutX(int y) { x = $one[[y]]; } }; _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
