kadircet added a comment.
The changes are not directly related to clang-tidy, so could you get rid of
that clang-tidy some-checker-specific tests and put some unittests into
clang/unittests/Format/CleanupTest.cpp ?
================
Comment at: clang/include/clang/Basic/CharInfo.h:196
+/// Requires that BufferPtr point to a newline character ("/n" or "/r").
+/// Returns a pointer past the end of any platform newline, i.e. past
----------------
use '\' instead of '/' ?
================
Comment at: clang/include/clang/Basic/CharInfo.h:200
+LLVM_READONLY inline const char *skipNewlineChars(const char *BufferPtr,
+ const char *BufferEnd) {
+ if (BufferPtr == BufferEnd)
----------------
formatting seems to be off, have you run clang-format on your changes?
================
Comment at: clang/include/clang/Basic/CharInfo.h:216
+/// Returns true if and only if S consists entirely of whitespace.
+LLVM_READONLY inline bool isWhitespaceStringRef(llvm::StringRef S) {
+ for (StringRef::const_iterator I = S.begin(), E = S.end(); I != E; ++I) {
----------------
could you make rename this to `isWhitespace` and move it closer to
`isWhitespace(unsigned char c)`
================
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) {
----------------
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?
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
