poelmanc marked 5 inline comments as done.
poelmanc added inline comments.
================
Comment at: clang/lib/AST/CommentParser.cpp:19
-static inline bool isWhitespace(llvm::StringRef S) {
+// Consider moving this useful function to a more general utility location.
+bool isWhitespace(llvm::StringRef S) {
----------------
kadircet wrote:
> poelmanc wrote:
> > gribozavr2 wrote:
> > > clang/include/clang/Basic/CharInfo.h ?
> > Done. I renamed it to `isWhitespaceStringRef` to avoid making it an
> > overload of the existing `isWhitespace(unsigned char)`, which causes
> > ambiguous overload errors at QueryParser.cpp:42 & CommentSema.cpp:237.
> >
> > We could alternatively keep this as an `isWhitespace` overload and instead
> > change those two lines to use a `static_cast<bool (*)(unsigned
> > char)>(&clang::isWhitespace)` or precede them with a line like:
> > ```
> > bool (*isWhitespaceOverload)(unsigned char) = &clang::isWhitespace;
> > ```
> ah that's unfortunate, I believe it makes sense to have this as an overload
> though.
>
> Could you instead make the predicate explicit by putting
> ```[](char c) { return clang::isWhitespace(c); }```
> instead of just `clang::isWhitespace` in those call sites?
Excellent! Better than the two ideas I thought of. Thanks!
Repository:
rCTE Clang Tools Extra
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68682/new/
https://reviews.llvm.org/D68682
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
