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

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added inline comments.


================
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);
----------------
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?


================
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"}}
+}
----------------
Incidentally, you can put `// expected-error at -1{{...}}` on the following line, to avoid such long lines.


================
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}}
----------------
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?


Repository:
  rC Clang

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

https://reviews.llvm.org/D54903