sammccall added inline comments.
================
Comment at: clang-tools-extra/clangd/refactor/Rename.cpp:473-488
+static bool mayBeValidIdentifier(llvm::StringRef Ident) {
+ assert(llvm::json::isUTF8(Ident));
+ if (Ident.empty())
+ return false;
+ // We don't check all the rules for non-ascii characters (most are allowed).
+ bool AllowDollar = true; // lenient
+ if (llvm::isASCII(Ident.front()) &&
----------------
njames93 wrote:
> What's wrong with `isValidIdentifier` in `CharInfo.h`.
> Also isIdentifier(Body|Head) cover isASCII.
Many (most?) non-ascii characters *are* allowed.
isValidIdentifier & friends return `false` for non-ascii characters. This is OK
for the fast-path of the parser, where false negatives are OK and fall back to
the slow path.
We want the opposite bias: false positives are OK (allow some incorrect renames
involving unicode characters) but false negatives are not (reject some valid
renames)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D98424/new/
https://reviews.llvm.org/D98424
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
