On Fri, Apr 15, 2016 at 12:56 AM, Serge Pavlov via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> 2016-04-14 2:44 GMT+06:00 Richard Smith <rich...@metafoo.co.uk>:
>
>> rsmith added a comment.
>>
>> 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.
>>
>> New format was not a good idea. Removed it.
>
>
>>
>> ================
>> Comment at: lib/AST/Decl.cpp:1423
>> @@ +1422,3 @@
>> +///
>> +/// is printed as C1<T,N>::C2<T1,TT<>>::meth<T2> rather than
>> C1::C2::meth.
>> +///
>> ----------------
>> Presumably you mean `C1<T, Val>::C2<T1, TC>::meth<T2>`?
>>
>> Fixed.
>
>
>> ================
>> Comment at: lib/AST/Decl.cpp:1447
>> @@ +1446,3 @@
>> + OS << "...";
>> + OS << 'T';
>> + if (TypePNo)
>> ----------------
>> 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.
>>
>> ================
>> Comment at: lib/AST/Decl.cpp:1464
>> @@ +1463,3 @@
>> + OS << TemplPNo;
>> + OS << "<>";
>> + TemplPNo++;
>> ----------------
>> This doesn't make sense: a template template argument is passed as a
>> template, but adding `<>` would make it a type instead. Remove this.
>>
>> Both fixed.
>
>
>> ================
>> Comment at: lib/Sema/SemaTemplateInstantiateDecl.cpp:3611
>> @@ +3610,3 @@
>> + if (AtEndOfTU && !getDiagnostics().hasErrorOccurred()) {
>> + Diag(PointOfInstantiation, diag::warn_func_template_missing)
>> + << Function << PatternDecl;
>> ----------------
>> 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:
>>
>> warning: instantiation for 'X<int>::f<double>' is required but no
>> definition is available
>> X<int>::f(0.0);
>> ^
>> note: forward declaration of template entity is here
>> template<typename T> void f(T);
>> ^
>> note: add an explicit instantiation declaration to suppress this
>> warning if 'X<int>::f<double>' is explicitly instantiated in another
>> translation unit
>>
>>
>> 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.
>
> After the last changes big part of the patch do not refer to undefined
> templates directly.
>
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.
> May be it is worth splitting the patch into one that implements printing
> template parameters and another that implements warnings on undefined
> templates?
>
> Thanks,
> --Serge
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
