[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

[Corentin Jabot via Phabricator via llvm-commits]

cor3ntin added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:69
+  // Only insert spaces if there aren't already spaces between operators
+  StringRef SpaceBefore = std::isspace(lookahead(SM, Loc, -1)) ? "" : " ";
+  StringRef SpaceAfter =
----------------
aaron.ballman wrote:
> Hrmm, I feel like `std::isspace()` is not going to properly handle all the Unicode whitespace, but I don't think we handle them all anyway as I notice the lexer is using `isHorizontalWhitespace()`, `isVerticalWhitespace()`, and `isWhitespace()` from `CharInfo.h`. Maybe we should use the same utility here just to match the lexer? (I don't feel strongly, just tickled my spidey senses.)
I just saw this comment randomly in my mail.
I agree we should not use `isspace` - it's also probably less efficient because of locale. And we don't want lexing to depend on the host locale.
And makes it easier to extend the set of supported whitespace if we ever do thatt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107294