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

[Sam McCall via Phabricator via cfe-commits]

sammccall accepted this revision.
sammccall added a comment.
This revision is now accepted and ready to land.
Herald added a subscriber: cfe-commits.

Just a few optional ideas, this is fine as-is



================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:108
+    // Allow the comment to contain spaces and optionally an "=" at the end.
+    if (getPrecedingComment(Arg).trim().rtrim("=").trim() == ParamName)
+      return false;
----------------
nit: one of these trim()s can be an rtrim


================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:122
+
+    StringRef SourcePrefix = sourcePrefix(ExprStartLoc, SM);
+    // Trim whitespace preceding expression.
----------------
In order to use sourcePrefix it we're doing the buffer lookup stuff for every param.

Would be a little more direct to grab the mainfile buffer + fileid at the start, and then here just decompose the top macrocallerloc straight away.

Seems a little simpler to reason about, no idea if the performance matters much, up to you.


================
Comment at: clang-tools-extra/clangd/InlayHints.cpp:131
+    // Find start of comment and get contents.
+    return SourcePrefix.substr(SourcePrefix.rfind("/*") + 2);
+  }
----------------
I think we should handle the case where we never find `/*`. *maybe* it can't happen, but I wouldn't want to bet too much on it.

Alternatively, rewinding back to the start of the (possibly long) comment just to check if it matches a fixed string seems a little roundabout.

Could be:
```
if (!SourcePrefix.consume_back("*/")) return false;
SourcePrefix = SourcePrefix.rtrim().rtrim('=').rtrim();
if (!SourcePrefix.consume_back(ParamName)) return false;
return SourcePrefix.rtrim().endswith("/*");
```


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D100715/new/

https://reviews.llvm.org/D100715