[PATCH] D54903: [Sema] Improve static_assert diagnostics.

Clement Courbet via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Nov 29 05:13:50 PST 2018


courbet added a comment.

Thanks



================
Comment at: lib/Sema/SemaTemplate.cpp:3064
+    // If this is a qualified name, expand the template arguments in nested
+    // qualifiers.
+    DR->getQualifier()->print(OS, PrintPolicy, true);
----------------
Quuxplusone wrote:
> I don't understand this code enough to say whether this "qualified name" business is needed. Is this what currently prevents you from expanding e.g. `is_base_of_v<T, U>`, as opposed to `is_base_of<T, U>::value`? What would go wrong if you removed //this// condition?
Expr does not have getQualifier(), DeclRefExpr is the one that does.

is_base_of_v is also a DeclRefExpr, so that works as expected.


================
Comment at: test/SemaCXX/static-assert.cpp:123
+void foo() {
+  static_assert(std::is_same<RAI_tag, typename Container::iterator::tag>::value, "message"); // expected-error{{static_assert failed due to requirement 'std::is_same<RAI_tag, BI_tag>::value' "message"}}
+}
----------------
Quuxplusone wrote:
> Incidentally, you can put `// expected-error at -1{{...}}` on the following line, to avoid such long lines.
Thanks for the tip.


================
Comment at: test/SemaCXX/static-assert.cpp:141
+}
+template void foo2<int, float, 3>(); // expected-note {{in instantiation of function template specialization 'foo2<int, float, 3>' requested here}}
----------------
Quuxplusone wrote:
> Nice!
> I would also like to see at least one test where one or more of the types are hard-to-name; e.g.
> ```
> template<class T>
> void foo(T t) {
>     static_assert(std::is_integral<T>::value);
>     static_assert(std::is_integral<decltype(t)>::value);
> }
> void test() { foo([](){}); }
> ```
> Another interesting case would be where one of the types is non-existent — which I //expect// would not hit your codepath at all, right?
> ```
> template<class T>
> void foo(T t) {
>     static_assert(std::is_integral<typename T::iterator>::value);
>     static_assert(std::is_integral<decltype(*t)>::value);
> }
> void test() { foo(42); }
> ```
> And should there be any test for a `static_assert` with no "message" element? or are you confident that that'll just work, and you want to keep this test file building in C++11 mode?
> I would also like to see at least one test where one or more of the types are hard-to-name; e.g.

SG, done.

> Another interesting case would be where one of the types is non-existent — which I expect would not hit your codepath at all, right?

Right. Done.

> And should there be any test for a static_assert with no "message" element? or are you confident that that'll just work, and you want to keep this test file building in C++11 mode?

This test file has negative tests for c++17 message-less static asserts, so I've added the c++17 tests in a separate file.


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903





More information about the cfe-commits mailing list