[PATCH] D100715: [clangd] Omit parameter hint if parameter name comment is present

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 340391.
nridge marked 3 inline comments as done.
nridge added a comment.

Address review comments


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) {


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D100715.340391.patch
Type: text/x-patch
Size: 3355 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20210425/a48de053/attachment.bin>