courbet marked 7 inline comments as done.
courbet added inline comments.
================
Comment at: lib/Sema/SemaTemplate.cpp:3065
+
+ ~FailedBooleanConditionPrinterHelper() override {}
+
----------------
Quuxplusone wrote:
> 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.
I don't think the style guide says anything; removed.
================
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"}}
----------------
Quuxplusone wrote:
> 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");
> ```
> Please also add a test case for the is_const_v inline-variable-template
> version.
done (in c++17 tests)
> Also, does using the PrinterHelper mean that you get a bunch of other cases
> for free? Like, does this work now too?
Exactly. While I'm at it I've added tests for your use case and removed the no
longer needed AllowTopLevel parameter.
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
