<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Fri, Apr 15, 2016 at 12:56 AM, Serge Pavlov via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><span class="">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></span><div>New format was not a good idea. Removed it.</div><span class=""><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></span><div>Fixed.</div><span class=""><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></span><div>Both fixed.</div><span class=""><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></span><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.</div></div></div></div></blockquote><div><br></div><div>My suggested wording said "template entity", which is the (not quite standard yet) term for a template or class/function/whatever declared within a template.</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div>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>
<br>_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
<br></blockquote></div><br></div></div>