This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG753b247d71d7: [clangd] Omit parameter hint if parameter name
comment is present (authored by nridge).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D100715/new/
https://reviews.llvm.org/D100715
Files:
clang-tools-extra/clangd/InlayHints.cpp
clang-tools-extra/clangd/unittests/InlayHintTests.cpp
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -322,6 +322,20 @@
)cpp");
}
+TEST(ParameterHints, ParamNameComment) {
+ // Do not hint an argument which already has a comment
+ // with the parameter name preceding it.
+ assertParameterHints(R"cpp(
+ void foo(int param);
+ void bar() {
+ foo(/*param*/42);
+ foo( /* param = */ 42);
+ foo(/* the answer */$param[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"param: ", "param"});
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -19,7 +19,13 @@
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
public:
InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST)
- : Results(Results), AST(AST.getASTContext()) {}
+ : Results(Results), AST(AST.getASTContext()),
+ MainFileID(AST.getSourceManager().getMainFileID()) {
+ bool Invalid = false;
+ llvm::StringRef Buf =
+ AST.getSourceManager().getBufferData(MainFileID, &Invalid);
+ MainFileBuf = Invalid ? StringRef{} : Buf;
+ }
bool VisitCXXConstructExpr(CXXConstructExpr *E) {
// Weed out constructor calls that don't look like a function call with
@@ -102,11 +108,37 @@
if (ParamName == getSpelledIdentifier(Arg))
return false;
- // FIXME: Exclude argument expressions preceded by a /*paramName=*/
comment.
+ // Exclude argument expressions preceded by a /*paramName*/.
+ if (isPrecededByParamNameComment(Arg, ParamName))
+ return false;
return true;
}
+ // Checks if "E" is spelled in the main file and preceded by a C-style
comment
+ // whose contents match ParamName (allowing for whitespace and an optional
"="
+ // at the end.
+ bool isPrecededByParamNameComment(const Expr *E, StringRef ParamName) {
+ auto &SM = AST.getSourceManager();
+ auto ExprStartLoc = SM.getTopMacroCallerLoc(E->getBeginLoc());
+ auto Decomposed = SM.getDecomposedLoc(ExprStartLoc);
+ if (Decomposed.first != MainFileID)
+ return false;
+
+ StringRef SourcePrefix = MainFileBuf.substr(0, Decomposed.second);
+ // Allow whitespace between comment and expression.
+ SourcePrefix = SourcePrefix.rtrim();
+ // Check for comment ending.
+ if (!SourcePrefix.consume_back("*/"))
+ return false;
+ // Allow whitespace and "=" at end of comment.
+ SourcePrefix = SourcePrefix.rtrim().rtrim('=').rtrim();
+ // Other than that, the comment must contain exactly ParamName.
+ if (!SourcePrefix.consume_back(ParamName))
+ return false;
+ return SourcePrefix.rtrim().endswith("/*");
+ }
+
// If "E" spells a single unqualified identifier, return that name.
// Otherwise, return an empty string.
static StringRef getSpelledIdentifier(const Expr *E) {
@@ -208,6 +240,8 @@
std::vector<InlayHint> &Results;
ASTContext &AST;
+ FileID MainFileID;
+ StringRef MainFileBuf;
};
std::vector<InlayHint> inlayHints(ParsedAST &AST) {
Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===================================================================
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -322,6 +322,20 @@
)cpp");
}
+TEST(ParameterHints, ParamNameComment) {
+ // Do not hint an argument which already has a comment
+ // with the parameter name preceding it.
+ assertParameterHints(R"cpp(
+ void foo(int param);
+ void bar() {
+ foo(/*param*/42);
+ foo( /* param = */ 42);
+ foo(/* the answer */$param[[42]]);
+ }
+ )cpp",
+ ExpectedHint{"param: ", "param"});
+}
+
} // namespace
} // namespace clangd
} // namespace clang
Index: clang-tools-extra/clangd/InlayHints.cpp
===================================================================
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -19,7 +19,13 @@
class InlayHintVisitor : public RecursiveASTVisitor<InlayHintVisitor> {
public:
InlayHintVisitor(std::vector<InlayHint> &Results, ParsedAST &AST)
- : Results(Results), AST(AST.getASTContext()) {}
+ : Results(Results), AST(AST.getASTContext()),
+ MainFileID(AST.getSourceManager().getMainFileID()) {
+ bool Invalid = false;
+ llvm::StringRef Buf =
+ AST.getSourceManager().getBufferData(MainFileID, &Invalid);
+ MainFileBuf = Invalid ? StringRef{} : Buf;
+ }
bool VisitCXXConstructExpr(CXXConstructExpr *E) {
// Weed out constructor calls that don't look like a function call with
@@ -102,11 +108,37 @@
if (ParamName == getSpelledIdentifier(Arg))
return false;
- // FIXME: Exclude argument expressions preceded by a /*paramName=*/ comment.
+ // Exclude argument expressions preceded by a /*paramName*/.
+ if (isPrecededByParamNameComment(Arg, ParamName))
+ return false;
return true;
}
+ // Checks if "E" is spelled in the main file and preceded by a C-style comment
+ // whose contents match ParamName (allowing for whitespace and an optional "="
+ // at the end.
+ bool isPrecededByParamNameComment(const Expr *E, StringRef ParamName) {
+ auto &SM = AST.getSourceManager();
+ auto ExprStartLoc = SM.getTopMacroCallerLoc(E->getBeginLoc());
+ auto Decomposed = SM.getDecomposedLoc(ExprStartLoc);
+ if (Decomposed.first != MainFileID)
+ return false;
+
+ StringRef SourcePrefix = MainFileBuf.substr(0, Decomposed.second);
+ // Allow whitespace between comment and expression.
+ SourcePrefix = SourcePrefix.rtrim();
+ // Check for comment ending.
+ if (!SourcePrefix.consume_back("*/"))
+ return false;
+ // Allow whitespace and "=" at end of comment.
+ SourcePrefix = SourcePrefix.rtrim().rtrim('=').rtrim();
+ // Other than that, the comment must contain exactly ParamName.
+ if (!SourcePrefix.consume_back(ParamName))
+ return false;
+ return SourcePrefix.rtrim().endswith("/*");
+ }
+
// If "E" spells a single unqualified identifier, return that name.
// Otherwise, return an empty string.
static StringRef getSpelledIdentifier(const Expr *E) {
@@ -208,6 +240,8 @@
std::vector<InlayHint> &Results;
ASTContext &AST;
+ FileID MainFileID;
+ StringRef MainFileBuf;
};
std::vector<InlayHint> inlayHints(ParsedAST &AST) {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
