cor3ntin added inline comments.
================ Comment at: clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:85 + assert(First != nullptr); + if (std::isalpha(*First) || Loc.isMacroID()) + return; ---------------- cjdb wrote: > aaron.ballman wrote: > > `isLetter()`? (or potentially `isIdentifierHead()` if we need to care about > > dollar-sign prefixed identifiers) > Is this necessary in the given context? The reason I was confident in using > `isalpha` is because we know this is an operator and the only values that > this could possibly be are one of `a`, `b`, `c`, `n`, `o`, or `x`. > > (cc @cor3ntin) Definitively. Both because trying to make assumption based on the current set of leading symbols in alternative token seems brittle to me. But mostly because `isalpha` answers "is this byte interpreted in the encoding associated to the current locale a letter". in some scenario (clang running on zOS for example) this will never give the right answer, I agree with Aaron: use `isIdentifierHead()` But maybe I'd go further: Have you considered first checking for the opcode first, then the spelling? 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
