[PATCH] D128762: [Clang] Rename StringLiteral::isAscii() => isOrdinary() [NFC]

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Jun 29 08:01:04 PDT 2022


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

Assuming precommit CI comes back clean, this LGTM!



================
Comment at: clang/include/clang/AST/Expr.h:1789-1886
+  enum StringKind { Ordinary, Wide, UTF8, UTF16, UTF32 };
 
 private:
   unsigned numTrailingObjects(OverloadToken<unsigned>) const { return 1; }
   unsigned numTrailingObjects(OverloadToken<SourceLocation>) const {
     return getNumConcatenated();
   }
----------------
cor3ntin wrote:
> aaron.ballman wrote:
> > I like the idea of renaming both of these -- `isASCII()` can be a bit confusing with `containsNonAscii()` as well. However, instead of "ordinary", would it make more sense to go with "narrow" as the term of art? That also goes nicely with the use of "wide" in this interface.
> narrow encompass ordinary and `u8` literals in standard terminology.
> And, I think `narrow` is too easilly mistaken as a synonym for `getByteLength() == 1`
Ah, okay, I didn't remember correctly that narrow included UTF-8 literals. "Ordinary" is fine by me, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128762



More information about the cfe-commits mailing list