[PATCH] D55933: [Sema] Do not print default template parameters.

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

Quuxplusone added inline comments.


================
Comment at: lib/AST/TypePrinter.cpp:173
+  // information about whether default template parameters were actually
+  // written.
+  switch (QT.getTypePtr()->getTypeClass()) {
----------------
FWIW, I don't understand this comment's terminology. "Whether default template parameters were actually written": should that say something like "whether a template parameter came from a default argument"?
And why are the `Pointer`, `VariableArray`, etc. cases here? What goes wrong if you remove them?


================
Comment at: test/SemaCXX/static-assert-cxx17.cpp:99
   static_assert((const X<typename T::T>[]){} == nullptr);
-  // expected-error at -1{{static_assert failed due to requirement '(X<int> const[0]){} == nullptr'}}
+  // expected-error at -1{{static_assert failed due to requirement '(const X<int> [0]){} == nullptr'}}
   static_assert(sizeof(X<decltype(X<typename T::T>().X<typename T::T>::~X())>) == 0);
----------------
(1) This cosmetic change is produced by the `case IncompleteArray:` that I questioned above, right? I'm surprised that the pretty-printed type changes from "east const" to "west const." But that's fine.

(2) Sorry I didn't notice before: surely where the message currently says `[0]` it should say `[]` instead! That's an existing bug in the pretty-printer, going back super far: https://godbolt.org/z/y9KzEq  So I guess it's not your responsibility to fix. But I hope there's a bug open about it somewhere?


================
Comment at: test/SemaCXX/static-assert-cxx17.cpp:103
+  static_assert(constexpr_return_false<typename T::T>());
+  // expected-error at -1{{static_assert failed due to requirement 'constexpr_return_false<int>()'}}
 }
----------------
Please keep the old test case 

    static_assert(constexpr_return_false<typename T::T, typename T::U>());
    // expected-error at -1{{static_assert failed due to requirement 'constexpr_return_false<int, float>()'}}

and then //add// this new test case. My understanding is that the old test case should still pass, even after your change.


================
Comment at: test/SemaCXX/static-assert-cxx17.cpp:109
+template <class T, class U>
+void foo7() {
+  static_assert(X<typename T::T, U>());
----------------
None of these new test cases actually involve the default template argument.

I'd like to see two test cases explicitly mimicking `vector`. (Warning: I didn't run this code!)
```
template<class> struct A {};
template<class, class = A<T>> struct V {};
template<class C> void testx() {
    static_assert(std::is_same<C*, void>::value, "");
    // expected-error at -1{{due to requirement 'std::is_same<V<int>*, void>::value'}}
}
template testx<V<int>>();
template<class> struct PMRA {};
template<class T> using PMRV = V<T, PMRA<T>>;
template<class C> void test() {
    static_assert(std::is_same<C*, void>::value, "");
    // expected-error at -1{{due to requirement 'std::is_same<V<int, PMRA<int>>*, void>::value'}}
    // expected-error at -2{{due to requirement 'std::is_same<PMRV<int>*, void>::value'}}
}
template testy<PMRV<int>>();
```
The `expected-error at -2` above is my fantasy world. I don't think it's possible to achieve in real life. :)


================
Comment at: test/SemaCXX/static-assert.cpp:130
 static_assert(std::is_same<decltype(std::is_const<const ExampleTypes::T>()), int>::value, "message");
-// expected-error at -1{{static_assert failed due to requirement 'std::is_same<std::is_const<const int>, int>::value' "message"}}
+// expected-error at -1{{static_assert failed due to requirement 'std::is_same<is_const<const int>, int>::value' "message"}}
 static_assert(std::is_const<decltype(ExampleTypes::T(3))>::value, "message");
----------------
Why did this output change?


Repository:
  rC Clang

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

https://reviews.llvm.org/D55933