[PATCH] D54324: [AST] Store the value of CharacterLiteral in the bit-fields of Stmt if possible

Shafik Yaghmour via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 15 10:14:22 PST 2018


shafik added inline comments.


================
Comment at: include/clang/AST/Expr.h:1407
 public:
-  enum CharacterKind {
-    Ascii,
-    Wide,
-    UTF8,
-    UTF16,
-    UTF32
-  };
+  enum CharacterKind { Ascii, Wide, UTF8, UTF16, UTF32 };
 
----------------
riccibruno wrote:
> shafik wrote:
> > Minor comment, does it make sense to covert this to a scoped enum since it looks like it is being strictly used as a set of values.
> Does it really add anything ?
> It means that instead of writing `CharacterLiteral::UTF32`
> you have to write `CharacterLiteral::CharacterKind::UTF32`
> 
> Seems a bit verbose. But I don't have any strong opinion on this.
It adds the protection against unintended conversions to and from the scoped enum, which in general is a good thing. The other benefits is not having the enumerators leak out into the surrounding scope but is limited in this case.

It does add verbosity though.


Repository:
  rC Clang

https://reviews.llvm.org/D54324





More information about the cfe-commits mailing list