Quuxplusone added inline comments.
================
Comment at: lib/Sema/SemaTemplate.cpp:3062
+public:
+ FailedBooleanConditionPrinterHelper(const PrintingPolicy &Policy)
+ : Policy(Policy) {}
----------------
`explicit`
================
Comment at: lib/Sema/SemaTemplate.cpp:3065
+
+ ~FailedBooleanConditionPrinterHelper() override {}
+
----------------
aaron.ballman wrote:
> Is this definition necessary?
Nit: I don't know if it is LLVM style to provide explicitly written-out
overrides for all virtual destructors. In my own code, if a destructor would be
redundant, I wouldn't write it.
================
Comment at: lib/Sema/SemaTemplate.cpp:3089
+
+} // namespace
----------------
`} // end anonymous namespace`
================
Comment at: test/SemaCXX/static-assert.cpp:117
+static_assert(!(std::is_const<const ExampleTypes::T>::value), "message");
+// expected-error@-1{{static_assert failed due to requirement
'!(std::is_const<const int>::value)' "message"}}
----------------
Please also add a test case for the `is_const_v` inline-variable-template
version.
```
template<class T>
inline constexpr bool is_const_v = is_const<T>::value;
static_assert(is_const_v<ExampleTypes::T>, "message"); // if this test case
was missing from the previous patch
static_assert(!is_const_v<ExampleTypes::ConstT>, "message"); // exercise the
same codepath for this new feature
```
Also, does using the PrinterHelper mean that you get a bunch of other cases for
free? Like, does this work now too?
```
static_assert(is_const<T>::value == false, "message");
```
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D55270/new/
https://reviews.llvm.org/D55270
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
