[PATCH] D68682: format::cleanupAroundReplacements removes whole line when Removals leave previously non-blank line blank

[Conrad Poelman via Phabricator via cfe-commits]

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