[PATCH] D114149: [clang-tidy] Fix pr48613: "llvm-header-guard uses a reserved identifier"

Mon Nov 29 12:24:40 PST 2021

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

In D114149#3144614 <https://reviews.llvm.org/D114149#3144614>, @salman-javed-nz wrote:

> I reverted my changes to do with the invalid character substitution. Doing something akin to `isAllowedInitiallyIDChar()` and `isAllowedIDChar()` in Lexer.cpp will require converting from `char*` to `UTF32*`. Windows complicates things by using its own code page (typically Windows-1252). Will require a lot of careful consideration to implement correctly.
>
> I have reverted this part of the patch for now, as it is not mentioned by the PR.

I think reverting the changes was a good idea -- we can handle that separately as it is more complex and this solves a real problem. However, I'm not certain that Windows complicates anything here as Clang accepts UTF-8 input. We have `-finput-charset` but that only supports UTF-8 currently.

LGTM, modulo a commenting nit.

================
Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:169

-    // Allow a trailing underscore iff we don't have to change the endif comment
     // too.
----------------
This wasn't actually a typo -- I think it meant "if and only if" (https://en.wikipedia.org/wiki/If_and_only_if). Feel free to expand the comment instead of using `iff`.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149