njames93 added a comment.
The for Pipe/BitwiseOr issue. One heuristic could be:
- If it has any kind of template dependence don't diagnost
- If its a `BinaryOperator`, its safe to diagnose.
- If its a `CxxOperatorCallExpr`, the simplest case I can think of is to check
the spelling of the operator (`operator|` vs `operator bitor`)
================
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 =
----------------
cor3ntin wrote:
> 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
Is it not just wiser to always add the spaces and just let clang-format do its
thing.
================
Comment at:
clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:117-120
+ case Opcode::BO_LAnd:
+ diag(Loc, "use 'and' for logical conjunctions")
+ << createReplacement(SM, Loc, "and", 2) << includeIso646(SM, Loc);
+ break;
----------------
aaron.ballman wrote:
> I think these can be replaced with a helper function along these lines:
> ```
> void emitDiag(const SourceManager &SM, SourceLocation Loc, StringRef Op,
> StringRef OpDesc, int Lookahead) {
> diag(Loc, ("use '" + Op + "' for " + OpDesc).str()) <<
> createReplacement(SM, Loc, Op, Lookahead) << includeIso646(SM, Loc);
> }
> ```
> WDYT?
```lang=c++
diag("use '%0' for %1") << Op << OpDesc << Fixes;
```
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D107294/new/
https://reviews.llvm.org/D107294
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
