[PATCH] D116203: [clang] adds unary type transformations as compiler built-ins

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Apr 18 07:39:11 PDT 2022

aaron.ballman added a comment.

Generally looking good to me, mostly just drive-by commentary at this point.

Comment at: clang/include/clang/Sema/DeclSpec.h:407
   static bool isTypeRep(TST T) {
-    return (T == TST_typename || T == TST_typeofType ||
-            T == TST_underlyingType || T == TST_atomic);
+    static const llvm::SmallVector<TST, 19> validTSTs = {
+        TST_atomic,
Should we find a way we can drop the `19` here so that this doesn't become incorrect when someone adds anything to `TransformTypeTraits.def`?

Comment at: clang/lib/Parse/ParseDecl.cpp:3421-3422
     case tok::kw_decltype:
     case tok::identifier: {
+    ParseIdentifier:
       // This identifier can only be a typedef name if we haven't already seen
I don't know why this offends my sensibilities, but it does. It seems like the label should be with the other (case) labels to my lizard brain, but the current way is also correct.

Comment at: clang/lib/Parse/ParseExpr.cpp:1044
                                // constant: enumeration-constant
+  ParseIdentifier:
     // Turn a potentially qualified name into a annot_typename or
Well, at least my sensibilities are consistent about pearl clutching, as this one also jumped out at me. :-D

Comment at: clang/lib/Parse/ParseExpr.cpp:1784
+  ExpectedExpression:
     NotCastExpr = true;
And yet this one didn't offend my sensibilities.  It's the curly braces that's doing it. :-D

Comment at: clang/lib/Parse/ParseStmt.cpp:186
   case tok::identifier: {
+  ParseIdentifier:
     Token Next = NextToken();

(Also, I'm a bit concerned that this feature requires so many `goto`s. Each one is reasonable in isolation, but we're adding a fair amount of total parsing complexity with those. That said, I don't have a better solution to suggest.)

  rG LLVM Github Monorepo



More information about the cfe-commits mailing list