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

[Aaron Ballman via Phabricator via cfe-commits]

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
   default:
+  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();
----------------
Gahh!

(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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116203