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

[Salman Javed via Phabricator via cfe-commits]

salman-javed-nz added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/utils/HeaderGuard.cpp:28
+/// Replace invalid characters in the identifier with '_'.
+static std::string replaceInvalidChars(StringRef Identifier) {
+  std::string Fixed{Identifier};
----------------
aaron.ballman wrote:
> This fixes some issues but I think it introduces new issues too. The old code would accept Unicode characters and pass them along as (valid) identifier characters. The new code will replace all the Unicode characters with `_` which means some people's header guards may go from useful to `_______________`. We should definitely add test cases for Unicode file names.
> 
> Ultimately, I think this functionality will want to use `UnicodeCharSets.h` to implement something along the lines of what's done in `isAllowedInitiallyIDChar()` and `isAllowedIDChar()` from Lexer.cpp.
`isAllowed*IDChar()`does exactly what I want, but is a static function in Lexer.cpp.
Should I be changing its declaration so that it's exposed via Lexer.h?
I don't know what the bar is to warrant changing core public Clang API. 

If I can just call the function directly from llvm-header-guard, I can avoid code duplication.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D114149