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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
