[C++ RFC/Patch] PR 39813

[C++ RFC/Patch] PR 39813

[Paolo Carlini]

Hi,

I'm having another look at this old bug, and I think it should be easy 
to fix, but there are a few details which I do not understand. 
Essentially the issue is about:

   struct B
   {
     template <typename T>
     void fn() { cout << __PRETTY_FUNCTION__ << endl; }
   };

   int main()
   {
     B().fn<int>();
   }

which outputs:

void B::fn() [with T = int]

whereas submitter expected:

void B::fn<T>() [with T = int]

The request makes sense to me, if only because otherwise it's mysterious 
where the T between square bracket is coming from. Submitter also 
noticed that ultimately the issue is due to the DECL_FRIEND_PSEUDO_* 
check in the conditional at the end of dump_function_name:

   if (DECL_TEMPLATE_INFO (t)
       && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
       && (TREE_CODE (DECL_TI_TEMPLATE (t)) != TEMPLATE_DECL
       || PRIMARY_TEMPLATE_P (DECL_TI_TEMPLATE (t))))
     dump_template_parms (pp, DECL_TEMPLATE_INFO (t), !DECL_USE_TEMPLATE 
(t),
                          flags);

Yesterday I double checked that indeed removing it still fixes the issue 
modulo trivial regressions (a few existing testcases need consistent 
adjustments). Still, there are details which I'm missing (eg, the name 
of the macro seems weird to me: it mentions friends (and friends are 
mentioned in its comment too) but isn't used only for DECL_FRIEND_P true 
and indeed it is true for the non-friend 'fn' in the snippet at issue). 
For sure so far I haven't been able to find cases where removing the 
check leads to a wrong error message...

Thanks!
Paolo.

Re: [C++ RFC/Patch] PR 39813

[Jason Merrill]

On 04/28/2015 06:59 AM, Paolo Carlini wrote:
>        && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
> the name
> of the macro seems weird to me: it mentions friends (and friends are
> mentioned in its comment too) but isn't used only for DECL_FRIEND_P true
> and indeed it is true for the non-friend 'fn' in the snippet at issue).

It shouldn't be true for fn, that's a bug.

Jason

Re: [C++ RFC/Patch] PR 39813

[Paolo Carlini]

Hi,

On 04/28/2015 02:45 PM, Jason Merrill wrote:
> On 04/28/2015 06:59 AM, Paolo Carlini wrote:
>>        && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
>> the name
>> of the macro seems weird to me: it mentions friends (and friends are
>> mentioned in its comment too) but isn't used only for DECL_FRIEND_P true
>> and indeed it is true for the non-friend 'fn' in the snippet at issue).
>
> It shouldn't be true for fn, that's a bug.
Ok, but then what? Its definition boils down to very basic macros:

#define DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION(DECL) \
   (DECL_TEMPLATE_INFO (DECL) && !DECL_USE_TEMPLATE (DECL))

It could be only matter of adding a DECL_FRIEND_P check at the beginning?

Paolo.

Re: [C++ RFC/Patch] PR 39813

[Jason Merrill]

On 04/28/2015 08:54 AM, Paolo Carlini wrote:
> Hi,
>
> On 04/28/2015 02:45 PM, Jason Merrill wrote:
>> On 04/28/2015 06:59 AM, Paolo Carlini wrote:
>>>        && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
>>> the name
>>> of the macro seems weird to me: it mentions friends (and friends are
>>> mentioned in its comment too) but isn't used only for DECL_FRIEND_P true
>>> and indeed it is true for the non-friend 'fn' in the snippet at issue).
>>
>> It shouldn't be true for fn, that's a bug.
> Ok, but then what? Its definition boils down to very basic macros:
>
> #define DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION(DECL) \
>    (DECL_TEMPLATE_INFO (DECL) && !DECL_USE_TEMPLATE (DECL))
>
> It could be only matter of adding a DECL_FRIEND_P check at the beginning?

Sure, that sounds likely.

Jason

Re: [C++ RFC/Patch] PR 39813

[Paolo Carlini]

[-- Attachment #1: Type: text/plain, Size: 1771 bytes --]

Hi,

On 04/28/2015 02:59 PM, Jason Merrill wrote:
> On 04/28/2015 08:54 AM, Paolo Carlini wrote:
>> Hi,
>>
>> On 04/28/2015 02:45 PM, Jason Merrill wrote:
>>> On 04/28/2015 06:59 AM, Paolo Carlini wrote:
>>>>        && !DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION (t)
>>>> the name
>>>> of the macro seems weird to me: it mentions friends (and friends are
>>>> mentioned in its comment too) but isn't used only for DECL_FRIEND_P 
>>>> true
>>>> and indeed it is true for the non-friend 'fn' in the snippet at 
>>>> issue).
>>>
>>> It shouldn't be true for fn, that's a bug.
>> Ok, but then what? Its definition boils down to very basic macros:
>>
>> #define DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION(DECL) \
>>    (DECL_TEMPLATE_INFO (DECL) && !DECL_USE_TEMPLATE (DECL))
>>
>> It could be only matter of adding a DECL_FRIEND_P check at the 
>> beginning?
> Sure, that sounds likely.
The simple idea almost works. The below however has an additional 
STRIP_TEMPLATE, which I added to avoid ICEs for testcases like 
g++.dg/template/friend26.C, where DECL_FRIEND_P of plain tmpl is false, 
whereas DECL_FRIEND_P of STRIP_TEMPLATE (tmpl) is true for the use of 
DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION in template_for_substitution. 
Does that make sense? Then we come to the two *remaining* regressions:

     g++.old-deja/g++.pt/friend10.C
     g++.old-deja/g++.pt/link1.C

which fail at link time. Those would disappear if I replace the already 
mentioned DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION in 
template_for_substitution with the old implemention of it, just checking 
DECL_TEMPLATE_INFO && !DECL_USE_TEMPLATE. Interestingly, note that with 
current (and old) clang too these two tests do nor link. I'm a bit stuck 
on this...

Thanks!
Paolo.

/////////////////////

[-- Attachment #2: patch_39813_draft --]
[-- Type: text/plain, Size: 4619 bytes --]

Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h	(revision 222531)
+++ cp/cp-tree.h	(working copy)
@@ -4062,7 +4062,8 @@ more_aggr_init_expr_args_p (const aggr_init_expr_a
    instantiated will not be a DECL_TEMPLATE_INSTANTIATION, but will be
    a DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION.  */
 #define DECL_FRIEND_PSEUDO_TEMPLATE_INSTANTIATION(DECL) \
-  (DECL_TEMPLATE_INFO (DECL) && !DECL_USE_TEMPLATE (DECL))
+  (DECL_FRIEND_P (STRIP_TEMPLATE (DECL)) && DECL_TEMPLATE_INFO (DECL) \
+   && !DECL_USE_TEMPLATE (DECL))
 
 /* Nonzero if DECL is a function generated from a function 'temploid',
    i.e. template, member of class template, or dependent friend.  */
Index: testsuite/g++.dg/diagnostic/bindings1.C
===================================================================
--- testsuite/g++.dg/diagnostic/bindings1.C	(revision 222531)
+++ testsuite/g++.dg/diagnostic/bindings1.C	(working copy)
@@ -10,7 +10,7 @@ struct x {typedef int type;};
 
 int main()
 {
-  if (strcmp (foo(x(), 3), "const char* foo(T, typename T::type) "
+  if (strcmp (foo(x(), 3), "const char* foo<T>(T, typename T::type) "
 	      "[with T = x; typename T::type = int]") == 0)
     return 0;
   else 
Index: testsuite/g++.dg/ext/pretty1.C
===================================================================
--- testsuite/g++.dg/ext/pretty1.C	(revision 222531)
+++ testsuite/g++.dg/ext/pretty1.C	(working copy)
@@ -60,8 +60,8 @@ __assert_fail (const char *cond, const char *file,
   abort ();
 }
 
-// { dg-final { scan-assembler "int bar\\(T\\).*with T = int" } }
+// { dg-final { scan-assembler "int bar\\<T\\>\\(T\\).*with T = int" } }
 // { dg-final { scan-assembler "top level" } }
 // { dg-final { scan-assembler "int main\\(\\)" } }
-// { dg-final { scan-assembler "int bar\\(T\\).*with T = double" } }
-// { dg-final { scan-assembler "int bar\\(T\\).*with T = unsigned char\*" } }
+// { dg-final { scan-assembler "int bar\\<T\\>\\(T\\).*with T = double" } }
+// { dg-final { scan-assembler "int bar\\<T\\>\\(T\\).*with T = unsigned char\*" } }
Index: testsuite/g++.dg/ext/pretty4.C
===================================================================
--- testsuite/g++.dg/ext/pretty4.C	(revision 0)
+++ testsuite/g++.dg/ext/pretty4.C	(working copy)
@@ -0,0 +1,19 @@
+// PR c++/39813
+// { dg-do run  }
+
+extern "C" int strcmp(const char*, const char*);
+
+struct B
+{
+  template <typename T>
+  const char* fn() { return __PRETTY_FUNCTION__; }
+};
+
+int main()
+{
+  if (strcmp (B().fn<int>(),
+	      "const char* B::fn<T>() [with T = int]") == 0)
+    return 0;
+  else
+    return 1;
+}
Index: testsuite/g++.dg/template/local1.C
===================================================================
--- testsuite/g++.dg/template/local1.C	(revision 222531)
+++ testsuite/g++.dg/template/local1.C	(working copy)
@@ -14,7 +14,7 @@ template<class T> void A::f()
   struct B
   {
     void g() {}
-    static int x;	// { dg-error "static.*int A::f\\(\\)::B::x" "" }
+    static int x;	// { dg-error "static.*int A::f\\<T\\>\\(\\)::B::x" "" }
   };
 }
 
Index: testsuite/g++.dg/warn/pr61945.C
===================================================================
--- testsuite/g++.dg/warn/pr61945.C	(revision 222531)
+++ testsuite/g++.dg/warn/pr61945.C	(working copy)
@@ -7,5 +7,5 @@ class A {
 };
 class B : A {
   template <typename>
-  void foo ();		// { dg-warning "by .B::foo\\(\\)." }
+  void foo ();		// { dg-warning "by .B::foo\\< \\<template-parameter-1-1\\> \\>\\(\\)." }
 };
Index: testsuite/g++.old-deja/g++.ext/pretty3.C
===================================================================
--- testsuite/g++.old-deja/g++.ext/pretty3.C	(revision 222531)
+++ testsuite/g++.old-deja/g++.ext/pretty3.C	(working copy)
@@ -20,7 +20,7 @@ template<class T> void f1 (T)
   
   if (strcmp (function, "f1"))
     bad = true;
-  if (strcmp (pretty, "void f1(T) [with T = float]")) // only for float instantiation
+  if (strcmp (pretty, "void f1<T>(T) [with T = float]")) // only for float instantiation
     bad = true;
 }
 
Index: testsuite/g++.old-deja/g++.pt/memtemp77.C
===================================================================
--- testsuite/g++.old-deja/g++.pt/memtemp77.C	(revision 222531)
+++ testsuite/g++.old-deja/g++.pt/memtemp77.C	(working copy)
@@ -19,7 +19,7 @@ const char* S3<char>::h(int) { return __PRETTY_FUN
 int main()
 {
   if (strcmp (S3<double>::h(7), 
-	      "static const char* S3<T>::h(U) [with U = int; T = double]") == 0)
+	      "static const char* S3<T>::h<U>(U) [with U = int; T = double]") == 0)
     return 0;
   else 
     return 1;