[libcxx-commits] [PATCH] D67052: Add reference type transformation builtins

[Eric Fiselier via Phabricator via libcxx-commits]

EricWF added inline comments.


================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1099
+
+void Parser::ParseAddReferenceTypeSpecifier(DeclSpec &DS) {
+  DeclSpec::TST ReferenceTransformTST = ReferenceTransformTokToDeclSpec();
----------------
I think this should be generalized and merged with `ParseUnderlyingTypeSpecifier`.

Maybe called `ParseTransformationTypeSpecifier`.


================
Comment at: clang/lib/Sema/SemaType.cpp:1612
     break;
+  case DeclSpec::TST_addLValueReferenceType:
+  case DeclSpec::TST_addRValueReferenceType:
----------------
This should be merged with the above case. 


================
Comment at: clang/lib/Sema/SemaType.cpp:5490
+      // TODO: Should we check something like "IsUnaryTypeTransfrom(DS.getTypeSpecTypeLoc())"?
+      // assert(DS.getTypeSpecType() == DeclSpec::TST_underlyingType);
       TL.setKWLoc(DS.getTypeSpecTypeLoc());
----------------
`DS.getTypeSpecType() >= DeclSpec::TST_underlyingType && DS.getTypeSpecType() <= TST_removeReferenceType` would correct this assertion. 


================
Comment at: clang/test/SemaCXX/add_reference.cpp:66
+static_assert(test<Bar<int>>::value, "");
+static_assert(test<Baz<int>>::value, "");
----------------
You should test a lot more types here.  reference types, const types, pointer types, function types, non-referenceable types.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67052