EricWF added a comment.
LGTM other than the inline comments.
I'll take a second pass at this once they're addressed.
Let's land the patch this week!
================
Comment at: clang/include/clang/Sema/DeclSpec.h:419
static bool isTypeRep(TST T) {
return (T == TST_typename || T == TST_typeofType ||
T == TST_underlyingType || T == TST_atomic||
----------------
Is the block of values for these identifiers contiguous? Can we work towards
writing this as `min <= x <= max`?
================
Comment at: clang/lib/Parse/ParseDeclCXX.cpp:1094
return DeclSpec::TST_removeReferenceType;
+ case tok::kw___remove_cv:
+ return DeclSpec::TST_removeCV;
----------------
It would be really cool if we could simplify these conversions to
`static_cast<DeclSpec::TST>(Tok.getKind() - base);`
That said, it's probably easier to land this patch as is.
================
Comment at: clang/lib/Sema/SemaType.cpp:8475
+ }
+ case UnaryTransformType::AddCV: {
+ QualType Underlying = BaseType.withConst().withVolatile();
----------------
I don't know that we need `add_foo`, because if I want to optimize my
type-trait, I'll just write `T const`.
Adding code to clang has a maintenance cost; and that's non-trivial. The
`remove_foo` traits add a lot of value, and that justifies the cost.
================
Comment at: clang/test/SemaCXX/add_cv.cpp:43
+ static const bool value =
+ __is_same(typename remove_const<T>::type, T) &&
+ __is_same(typename remove_const<const volatile T>::type, volatile T) &&
----------------
Clang tests normally test:
(A) The diagnostics that emit when people seriously misuse.
(B) Just some failures.
================
Comment at:
libcxx/test/libcxx/utilities/meta/stress_tests/stress_test_add_cv.sh.cpp:2
+//===----------------------------------------------------------------------===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM
Exceptions.
----------------
I think we probably want to keep the libc++ changes in a separate patch.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67588/new/
https://reviews.llvm.org/D67588
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
