[C++ PATCH] PR c++/69855

[C++ PATCH] PR c++/69855

[Ville Voutilainen]

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

Tested on Linux-PPC64. Comments very much welcomed on the change
to g++.old-deja/g++.pt/crash3.C, I'm not at all sure what that test
is trying to do; it looks like it may have never cared about the names
of the local functions, but rather about the fact that the function
bodies of the member functions of the class template are not instantiated
just because the member functions return types that are specializations
of the class template.

/cp
    PR c++/69855.
    name-lookup.c (pushdecl_maybe_friend_1): Push local function
    decls into the global scope after stripping template bits
    and setting DECL_ANTICIPATED.

/testsuite
    PR c++/69855.
    g++.dg/overload/69855.C: New.
    g++.old-deja/g++.law/missed-error2.C: Adjust.
    g++.old-deja/g++.pt/crash3.C: Likewise.

[-- Attachment #2: 69855.diff4 --]
[-- Type: application/octet-stream, Size: 2819 bytes --]

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 86d260c..e7c1a52 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -929,6 +929,19 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend)
 	      DECL_ANTICIPATED (t) = 1;
 	      DECL_HIDDEN_FRIEND_P (t) = 1;
 	    }
+
+	  // PR c++/69855
+	  if (TREE_CODE (x) == FUNCTION_DECL
+	      && DECL_LOCAL_FUNCTION_P (x)
+	      && !DECL_OMP_DECLARE_REDUCTION_P (x)
+	      && !type_dependent_expression_p (x))
+	    {
+	      tree t2 = copy_decl (t);
+	      DECL_USE_TEMPLATE (t2) = 0;
+	      DECL_TEMPLATE_INFO (t2) = NULL_TREE;
+	      DECL_ANTICIPATED (t2) = 1;
+	      push_overloaded_decl (t2, PUSH_GLOBAL, is_friend);
+	    }
 	}
 
       if (t != x || DECL_FUNCTION_TEMPLATE_P (t))
diff --git a/gcc/testsuite/g++.dg/overload/69855.C b/gcc/testsuite/g++.dg/overload/69855.C
new file mode 100644
index 0000000..43658ff
--- /dev/null
+++ b/gcc/testsuite/g++.dg/overload/69855.C
@@ -0,0 +1,52 @@
+// PR c++/69855
+// { dg-do compile }
+
+int get();
+void f() {
+  char get(); // { dg-error "ambiguating" }
+}
+
+int get2();
+char get2(int);
+void f2() {
+  char get2(); // { dg-error "ambiguating" }
+}
+
+char get3(int);
+void f3() {
+  char get3();
+}
+
+void f4() {
+  char get4();
+}
+int get4(); // { dg-error "ambiguating" }
+
+void get5();
+
+template <class T> struct X
+{
+  void g()
+  {
+    int get5(); // { dg-error "ambiguating" }
+  }
+};
+
+
+template <class T> struct X2
+{
+  void g()
+  {
+    int get6();
+  }
+};
+
+void get6(); // { dg-error "ambiguating" }
+
+
+
+
+
+
+
+
diff --git a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
index 42f70ae..26ae87d 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
@@ -25,9 +25,10 @@ int main() {
    foo(4, -37, 14.39, 14.38);
 }
 
-// 971006 we no longer give an error for this since we emit a hard error
-// about the declaration above
-static void foo(int i, int j, double x, double y) { 
+// 971006 we no longer gave an error for this since we emit a hard error
+// about the declaration above, but after the fix for PR c++/69855
+// this declaration emits a diagnostic again
+static void foo(int i, int j, double x, double y) { // { dg-error "extern|static" }
 
    std::cout << "Max(int): " << max(i,j) << " Max(double): " <<
 max(x,y) << '\n';
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash3.C b/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
index 160cbe5..2ba61d9 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
@@ -10,7 +10,7 @@ public:
     }
     CVector<long> g() const
     {
-       CVector<long> v();
-       return v;
+       CVector<long> v2();
+       return v2;
     }
 };

Re: [C++ PATCH] PR c++/69855

[Paolo Carlini]

.. minor nit: the new testcase has a number of trailing blank lines.

Paolo.

Re: [C++ PATCH] PR c++/69855

[Ville Voutilainen]

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

On 5 May 2016 at 13:36, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> .. minor nit: the new testcase has a number of trailing blank lines.


New patch attached. :)

[-- Attachment #2: 69855.diff5 --]
[-- Type: application/octet-stream, Size: 2803 bytes --]

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index 86d260c..e7c1a52 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -929,6 +929,19 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend)
 	      DECL_ANTICIPATED (t) = 1;
 	      DECL_HIDDEN_FRIEND_P (t) = 1;
 	    }
+
+	  // PR c++/69855
+	  if (TREE_CODE (x) == FUNCTION_DECL
+	      && DECL_LOCAL_FUNCTION_P (x)
+	      && !DECL_OMP_DECLARE_REDUCTION_P (x)
+	      && !type_dependent_expression_p (x))
+	    {
+	      tree t2 = copy_decl (t);
+	      DECL_USE_TEMPLATE (t2) = 0;
+	      DECL_TEMPLATE_INFO (t2) = NULL_TREE;
+	      DECL_ANTICIPATED (t2) = 1;
+	      push_overloaded_decl (t2, PUSH_GLOBAL, is_friend);
+	    }
 	}
 
       if (t != x || DECL_FUNCTION_TEMPLATE_P (t))
diff --git a/gcc/testsuite/g++.dg/overload/69855.C b/gcc/testsuite/g++.dg/overload/69855.C
new file mode 100644
index 0000000..dc2d733
--- /dev/null
+++ b/gcc/testsuite/g++.dg/overload/69855.C
@@ -0,0 +1,44 @@
+// PR c++/69855
+// { dg-do compile }
+
+int get();
+void f() {
+  char get(); // { dg-error "ambiguating" }
+}
+
+int get2();
+char get2(int);
+void f2() {
+  char get2(); // { dg-error "ambiguating" }
+}
+
+char get3(int);
+void f3() {
+  char get3();
+}
+
+void f4() {
+  char get4();
+}
+int get4(); // { dg-error "ambiguating" }
+
+void get5();
+
+template <class T> struct X
+{
+  void g()
+  {
+    int get5(); // { dg-error "ambiguating" }
+  }
+};
+
+
+template <class T> struct X2
+{
+  void g()
+  {
+    int get6();
+  }
+};
+
+void get6(); // { dg-error "ambiguating" }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
index 42f70ae..26ae87d 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
@@ -25,9 +25,10 @@ int main() {
    foo(4, -37, 14.39, 14.38);
 }
 
-// 971006 we no longer give an error for this since we emit a hard error
-// about the declaration above
-static void foo(int i, int j, double x, double y) { 
+// 971006 we no longer gave an error for this since we emit a hard error
+// about the declaration above, but after the fix for PR c++/69855
+// this declaration emits a diagnostic again
+static void foo(int i, int j, double x, double y) { // { dg-error "extern|static" }
 
    std::cout << "Max(int): " << max(i,j) << " Max(double): " <<
 max(x,y) << '\n';
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash3.C b/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
index 160cbe5..2ba61d9 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
@@ -10,7 +10,7 @@ public:
     }
     CVector<long> g() const
     {
-       CVector<long> v();
-       return v;
+       CVector<long> v2();
+       return v2;
     }
 };

Re: [C++ PATCH] PR c++/69855

[Jason Merrill]

On 05/05/2016 09:11 AM, Ville Voutilainen wrote:
> On 5 May 2016 at 13:36, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>> .. minor nit: the new testcase has a number of trailing blank lines.
>
> New patch attached. :)

Sorry for the delay.

Please use ".diff" for patches so that they are properly recognized as 
text/x-patch.

The patch looks good, but it could use a comment explaining what it's doing.

Any thoughts on doing something similar for extern variable declarations?

Jason

Re: [C++ PATCH] PR c++/69855

[Ville Voutilainen]

On 19 May 2016 at 19:40, Jason Merrill <jason@redhat.com> wrote:
> On 05/05/2016 09:11 AM, Ville Voutilainen wrote:
>>
>> On 5 May 2016 at 13:36, Paolo Carlini <paolo.carlini@oracle.com> wrote:
>>>
>>> .. minor nit: the new testcase has a number of trailing blank lines.
>>
>>
>> New patch attached. :)
>
>
> Sorry for the delay.
>
> Please use ".diff" for patches so that they are properly recognized as
> text/x-patch.
>
> The patch looks good, but it could use a comment explaining what it's doing.

I'll add some comments into it.

> Any thoughts on doing something similar for extern variable declarations?

Ah, we diagnose local extern variable declarations that clash with
previous declarations,
but we don't diagnose cases where a subsequent declaration clashes
with a previous
local extern declaration. I'll take a look.

Re: [C++ PATCH] PR c++/69855

[Ville Voutilainen]

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

On 20 May 2016 at 07:05, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
> On 19 May 2016 at 19:40, Jason Merrill <jason@redhat.com> wrote:
>> Any thoughts on doing something similar for extern variable declarations?
>
> Ah, we diagnose local extern variable declarations that clash with
> previous declarations,
> but we don't diagnose cases where a subsequent declaration clashes
> with a previous
> local extern declaration. I'll take a look.

As discussed on irc, this requires teaching variable declarations to
work with DECL_ANTICIPATED
and is thus some amounts of surgery, so the recommendation was to go
ahead with this patch.
I added a comment to the new code block, an updated patch attached.
Changelog as before.
Ok for trunk?

[-- Attachment #2: 69855_6.diff --]
[-- Type: text/plain, Size: 3066 bytes --]

diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index eb128db..568c75e 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -929,6 +929,24 @@ pushdecl_maybe_friend_1 (tree x, bool is_friend)
 	      DECL_ANTICIPATED (t) = 1;
 	      DECL_HIDDEN_FRIEND_P (t) = 1;
 	    }
+
+	  if (TREE_CODE (x) == FUNCTION_DECL
+	      && DECL_LOCAL_FUNCTION_P (x)
+	      && !DECL_OMP_DECLARE_REDUCTION_P (x)
+	      && !type_dependent_expression_p (x))
+	    {
+	      /* PR c++/69855, a local function declaration
+		 is stripped from template info and pushed to
+		 the local scope as a hidden declaration. This
+		 allows ill-formed overloads even in other scopes
+		 to be diagnosed both at the local declaration site
+		 and after it.  */
+	      tree t2 = copy_decl (t);
+	      DECL_USE_TEMPLATE (t2) = 0;
+	      DECL_TEMPLATE_INFO (t2) = NULL_TREE;
+	      DECL_ANTICIPATED (t2) = 1;
+	      push_overloaded_decl (t2, PUSH_GLOBAL, is_friend);
+	    }
 	}
 
       if (t != x || DECL_FUNCTION_TEMPLATE_P (t))
diff --git a/gcc/testsuite/g++.dg/overload/69855.C b/gcc/testsuite/g++.dg/overload/69855.C
new file mode 100644
index 0000000..dc2d733
--- /dev/null
+++ b/gcc/testsuite/g++.dg/overload/69855.C
@@ -0,0 +1,44 @@
+// PR c++/69855
+// { dg-do compile }
+
+int get();
+void f() {
+  char get(); // { dg-error "ambiguating" }
+}
+
+int get2();
+char get2(int);
+void f2() {
+  char get2(); // { dg-error "ambiguating" }
+}
+
+char get3(int);
+void f3() {
+  char get3();
+}
+
+void f4() {
+  char get4();
+}
+int get4(); // { dg-error "ambiguating" }
+
+void get5();
+
+template <class T> struct X
+{
+  void g()
+  {
+    int get5(); // { dg-error "ambiguating" }
+  }
+};
+
+
+template <class T> struct X2
+{
+  void g()
+  {
+    int get6();
+  }
+};
+
+void get6(); // { dg-error "ambiguating" }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
index 42f70ae..26ae87d 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
@@ -25,9 +25,10 @@ int main() {
    foo(4, -37, 14.39, 14.38);
 }
 
-// 971006 we no longer give an error for this since we emit a hard error
-// about the declaration above
-static void foo(int i, int j, double x, double y) { 
+// 971006 we no longer gave an error for this since we emit a hard error
+// about the declaration above, but after the fix for PR c++/69855
+// this declaration emits a diagnostic again
+static void foo(int i, int j, double x, double y) { // { dg-error "extern|static" }
 
    std::cout << "Max(int): " << max(i,j) << " Max(double): " <<
 max(x,y) << '\n';
diff --git a/gcc/testsuite/g++.old-deja/g++.pt/crash3.C b/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
index 160cbe5..2ba61d9 100644
--- a/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
+++ b/gcc/testsuite/g++.old-deja/g++.pt/crash3.C
@@ -10,7 +10,7 @@ public:
     }
     CVector<long> g() const
     {
-       CVector<long> v();
-       return v;
+       CVector<long> v2();
+       return v2;
     }
 };

Re: [C++ PATCH] PR c++/69855

[Jason Merrill]

OK, thanks.

Jason


On Fri, May 27, 2016 at 10:43 AM, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> On 20 May 2016 at 07:05, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
>> On 19 May 2016 at 19:40, Jason Merrill <jason@redhat.com> wrote:
>>> Any thoughts on doing something similar for extern variable declarations?
>>
>> Ah, we diagnose local extern variable declarations that clash with
>> previous declarations,
>> but we don't diagnose cases where a subsequent declaration clashes
>> with a previous
>> local extern declaration. I'll take a look.
>
> As discussed on irc, this requires teaching variable declarations to
> work with DECL_ANTICIPATED
> and is thus some amounts of surgery, so the recommendation was to go
> ahead with this patch.
> I added a comment to the new code block, an updated patch attached.
> Changelog as before.
> Ok for trunk?

Re: [C++ PATCH] PR c++/69855

[Ville Voutilainen]

On 27 May 2016 at 17:46, Jason Merrill <jason@redhat.com> wrote:
> OK, thanks.

Should this fix be backported to the gcc6-branch? I have no plans to
backport it any further than that.

>
> Jason
>
>
> On Fri, May 27, 2016 at 10:43 AM, Ville Voutilainen
> <ville.voutilainen@gmail.com> wrote:
>> On 20 May 2016 at 07:05, Ville Voutilainen <ville.voutilainen@gmail.com> wrote:
>>> On 19 May 2016 at 19:40, Jason Merrill <jason@redhat.com> wrote:
>>>> Any thoughts on doing something similar for extern variable declarations?
>>>
>>> Ah, we diagnose local extern variable declarations that clash with
>>> previous declarations,
>>> but we don't diagnose cases where a subsequent declaration clashes
>>> with a previous
>>> local extern declaration. I'll take a look.
>>
>> As discussed on irc, this requires teaching variable declarations to
>> work with DECL_ANTICIPATED
>> and is thus some amounts of surgery, so the recommendation was to go
>> ahead with this patch.
>> I added a comment to the new code block, an updated patch attached.
>> Changelog as before.
>> Ok for trunk?

Re: [C++ PATCH] PR c++/69855

[Jason Merrill]

On Fri, May 27, 2016 at 11:20 AM, Ville Voutilainen
<ville.voutilainen@gmail.com> wrote:
> On 27 May 2016 at 17:46, Jason Merrill <jason@redhat.com> wrote:
>> OK, thanks.

> Should this fix be backported to the gcc6-branch? I have no plans to
> backport it any further than that.

No, the bug isn't a regression and only affects invalid code, so it
isn't a good candidate for backporting.

Jason