<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">2016-04-14 2:44 GMT+06:00 Richard Smith <span dir="ltr"><<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">rsmith added a comment.<br>
<br>
I would prefer to avoid adding the `%qt` format if we can. But if we do provide it, the template parameter names we use should match what was written in the declaration of the template itself.<br>
<br></blockquote><div>New format was not a good idea. Removed it.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<br>
================<br>
Comment at: lib/AST/Decl.cpp:1423<br>
@@ +1422,3 @@<br>
+///<br>
+/// is printed as C1<T,N>::C2<T1,TT<>>::meth<T2> rather than C1::C2::meth.<br>
+///<br>
----------------<br>
Presumably you mean `C1<T, Val>::C2<T1, TC>::meth<T2>`?<br>
<br></blockquote><div>Fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: lib/AST/Decl.cpp:1447<br>
@@ +1446,3 @@<br>
+          OS << "...";<br>
+        OS << 'T';<br>
+        if (TypePNo)<br>
----------------<br>
Please don't invent a name here. If you really want this formatting for template names, please query the `TemplateDecl` to find the names that were used when declaring its parameters.<br>
<br>
================<br>
Comment at: lib/AST/Decl.cpp:1464<br>
@@ +1463,3 @@<br>
+          OS << TemplPNo;<br>
+        OS << "<>";<br>
+        TemplPNo++;<br>
----------------<br>
This doesn't make sense: a template template argument is passed as a template, but adding `<>` would make it a type instead. Remove this.<br>
<br></blockquote><div>Both fixed.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
================<br>
Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3611<br>
@@ +3610,3 @@<br>
+      if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {<br>
+        Diag(PointOfInstantiation, diag::warn_func_template_missing)<br>
+          << Function << PatternDecl;<br>
----------------<br>
Please produce a note pointing at the declaration of the undefined template. With a bit of rewording the diagnostics, I think you can remove the need for the `%qt` format entirely:<br>
<br>
  warning: instantiation for 'X<int>::f<double>' is required but no definition is available<br>
    X<int>::f(0.0);<br>
            ^<br>
  note: forward declaration of template entity is here<br>
    template<typename T> void f(T);<br>
                              ^<br>
  note: add an explicit instantiation declaration to suppress this warning if 'X<int>::f<double>' is explicitly instantiated in another translation unit<br>
<br>
<br></blockquote><div>If the purpose is to get rid of '%qt', probably the current solution (printing template parameters) fits? It seems that decorating templates with parameters can improve readability of messages.</div><div><br></div><div>After the last changes big part of the patch do not refer to undefined templates directly. May be it is worth splitting the patch into one that implements printing template parameters and another that implements warnings on undefined templates?</div><div><br></div><div>Thanks,</div><div>--Serge</div><div><br></div></div></div></div>