public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* [C++ Patch / RFC] PR 46206
@ 2013-08-05 22:46 Paolo Carlini
  2013-08-06  2:57 ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Carlini @ 2013-08-05 22:46 UTC (permalink / raw)
  To: gcc-patches; +Cc: Jason Merrill

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

Hi,

I have been investigating this very old and very weird issue where we 
wrongly reject:

class Foo
{
int u, v, w, x;
typedef struct Bar { } Bar;
virtual void foo(void) {
struct Bar bar;
}
};

46206.C: In member function ‘virtual void Foo::foo()’:
46206.C:6:12: error: using typedef-name ‘Foo::Bar’ after ‘struct’
46206.C:4:26: note: ‘Foo::Bar’ has a previous declaration here

whereas we don't reject variants with one less data member, say x, or 
non-virtual foo, all sorts of variants corresponding to a smaller Foo!!

I figured out that when we parse "typedef struct Bar { } Bar;" we create 
two TYPE_DECL: first, one marked as DECL_IMPLICIT_TYPEDEF_P in pushtag_1 
(via create_implicit_typedef); then a second, non-implicit, one in 
grokdeclarator, via build_lang_decl (TYPE_DECL... ). When we do lookup 
for "struct Bar bar", it happens that the *second* one is found, thus 
the check in check_elaborated_type_specifier triggers.

The latter function is called by lookup_and_check_tag, with the decl 
returned by lookup_name_prefer_type (name, 2). The latter, in turn, ends 
up calling lookup_name_real_1 which has:

/* If this is the kind of thing we're looking for, we're done. */
if (qualify_lookup (iter->value, flags))
binding = iter->value;
else if ((flags & LOOKUP_PREFER_TYPES)
&& qualify_lookup (iter->type, flags))
binding = iter->type;
else
binding = NULL_TREE;

and it happens that the first qualify_lookup succeeds but with 
iter->value which is the variant of the Bar TYPE_DECL with 
DECL_IMPLICIT_TYPEDEF_P not set. On the other hand, iter->type is the Ok 
variant, that which would not trigger the error.

Then I noticed the following comment in name_lookup.c, around line 4890, 
in lookup_type_scope_1:

We check ITER->TYPE before ITER->VALUE in order to handle
typedef struct C {} C;
correctly. */

and after this comment, both pairs of qualify_lookup are called in that 
order. Thus I started seriously suspecting that something may be wrong 
in the if-else above, that is, that we really want something with 
iter->type *before* iter->value there too: the attached patchlet p works 
for the testcase and passes bootstrap & test. Does it make sense to you?

Final observation: in many cases, like for example, variants of the 
testcase with one less data member, what happens is that iter->type and 
iter->value are *both* the same variant of the TYPE_DECL Bar, the one 
which is fine, has DECL_IMPLICIT_TYPEDEF_P set. Thus the ordering 
doesn't matter. Frankly, at the moment I'm not sure to understand how 
exactly when the class becomes bigger the iter->type and iter->value 
become different and becomes important to handle the former first.

Fiuuuu ;)

Thanks!
Paolo.

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

[-- Attachment #2: p --]
[-- Type: text/plain, Size: 687 bytes --]

Index: name-lookup.c
===================================================================
--- name-lookup.c	(revision 201511)
+++ name-lookup.c	(working copy)
@@ -4740,11 +4740,11 @@ lookup_name_real_1 (tree name, int prefer_type, in
 	  continue;
 
 	/* If this is the kind of thing we're looking for, we're done.  */
-	if (qualify_lookup (iter->value, flags))
+	if ((flags & LOOKUP_PREFER_TYPES)
+	    && qualify_lookup (iter->type, flags))
+	  binding = iter->type;
+	else if (qualify_lookup (iter->value, flags))
 	  binding = iter->value;
-	else if ((flags & LOOKUP_PREFER_TYPES)
-		 && qualify_lookup (iter->type, flags))
-	  binding = iter->type;
 	else
 	  binding = NULL_TREE;
 

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-05 22:46 [C++ Patch / RFC] PR 46206 Paolo Carlini
@ 2013-08-06  2:57 ` Jason Merrill
  2013-08-06  9:26   ` Paolo Carlini
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2013-08-06  2:57 UTC (permalink / raw)
  To: Paolo Carlini, gcc-patches

On 08/05/2013 06:46 PM, Paolo Carlini wrote:
> and after this comment, both pairs of qualify_lookup are called in that
> order. Thus I started seriously suspecting that something may be wrong
> in the if-else above, that is, that we really want something with
> iter->type *before* iter->value there too: the attached patchlet p works
> for the testcase and passes bootstrap & test. Does it make sense to you?

Yes.

> Final observation: in many cases, like for example, variants of the
> testcase with one less data member, what happens is that iter->type and
> iter->value are *both* the same variant of the TYPE_DECL Bar, the one
> which is fine, has DECL_IMPLICIT_TYPEDEF_P set.

That's strange.  I would expect that to mean that we don't properly give 
an error for a Bar data member declared after the typedef.

Jason

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-06  2:57 ` Jason Merrill
@ 2013-08-06  9:26   ` Paolo Carlini
  2013-08-06 16:35     ` Paolo Carlini
  2013-08-06 22:09     ` Jason Merrill
  0 siblings, 2 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-06  9:26 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi,

On 08/06/2013 04:57 AM, Jason Merrill wrote:
> On 08/05/2013 06:46 PM, Paolo Carlini wrote:
>> and after this comment, both pairs of qualify_lookup are called in that
>> order. Thus I started seriously suspecting that something may be wrong
>> in the if-else above, that is, that we really want something with
>> iter->type *before* iter->value there too: the attached patchlet p works
>> for the testcase and passes bootstrap & test. Does it make sense to you?
>
> Yes.
Ok, good.
>
>> Final observation: in many cases, like for example, variants of the
>> testcase with one less data member, what happens is that iter->type and
>> iter->value are *both* the same variant of the TYPE_DECL Bar, the one
>> which is fine, has DECL_IMPLICIT_TYPEDEF_P set.
>
> That's strange.  I would expect that to mean that we don't properly 
> give an error for a Bar data member declared after the typedef.
You mean something like this?

class Foo
{
   int u, v, w;//, x;
   typedef struct Bar { } Bar;
   Bar bar;
   virtual void foo(void) {
     struct Bar bar;
   }
};

after the patch we accept it, while we rejected it before. Current clang 
and icc also accept it. Note however, that in the above iter->type and 
iter->value are different, usual story. Same with w too commented out or 
neither. I haven't been able to construct a testcase following by and 
large your recipe where iter->type == iter->value. In short alternating 
data members and TYPE_DECLs also makes iter->type != iter->value, thus 
we want to check them in the proper order in such cases too.

Do you have more testcases in mind? (otherwise I would add the above too)

Thanks,
Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-06  9:26   ` Paolo Carlini
@ 2013-08-06 16:35     ` Paolo Carlini
  2013-08-06 22:09     ` Jason Merrill
  1 sibling, 0 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-06 16:35 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

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

... today I did the following: commented out the error at issue 
(decl.c:11828) and ran the testsuite. The attached js.txt is the list of 
fails. Wanted to make sure that we have enough coverage.

I'm also attaching here the complete patch + testcase which passes boot 
& test.

Thanks!
Paolo.

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

[-- Attachment #2: js.txt --]
[-- Type: text/plain, Size: 1203 bytes --]

FAIL: g++.dg/cpp0x/gen-attrs-27.C -std=c++11  (test for errors, line 6)
FAIL: g++.dg/ext/attrib27.C -std=c++98  (test for errors, line 5)
FAIL: g++.dg/ext/attrib27.C -std=c++11  (test for errors, line 5)
FAIL: g++.dg/lookup/struct1.C -std=c++98  (test for errors, line 6)
FAIL: g++.dg/lookup/struct1.C -std=c++98  (test for errors, line 9)
FAIL: g++.dg/lookup/struct1.C -std=c++11  (test for errors, line 6)
FAIL: g++.dg/lookup/struct1.C -std=c++11  (test for errors, line 9)
FAIL: g++.dg/parse/elab1.C -std=c++98  (test for errors, line 6)
FAIL: g++.dg/parse/elab1.C -std=c++11  (test for errors, line 6)
FAIL: g++.dg/parse/typedef3.C -std=c++98  (test for errors, line 7)
FAIL: g++.dg/parse/typedef3.C -std=c++11  (test for errors, line 7)
FAIL: g++.dg/template/crash26.C -std=c++98  (test for errors, line 8)
FAIL: g++.dg/template/crash26.C -std=c++11  (test for errors, line 8)

FAIL: g++.old-deja/g++.benjamin/typedef01.C -std=c++98  (test for errors, line 22)
FAIL: g++.old-deja/g++.benjamin/typedef01.C -std=c++11  (test for errors, line 22)
FAIL: g++.old-deja/g++.brendan/line1.C -std=c++98  (test for errors, line 4)
FAIL: g++.old-deja/g++.brendan/line1.C -std=c++11  (test for errors, line 4)

[-- Attachment #3: patch_46206 --]
[-- Type: text/plain, Size: 1205 bytes --]

Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 201524)
+++ cp/name-lookup.c	(working copy)
@@ -4740,11 +4740,11 @@ lookup_name_real_1 (tree name, int prefer_type, in
 	  continue;
 
 	/* If this is the kind of thing we're looking for, we're done.  */
-	if (qualify_lookup (iter->value, flags))
+	if ((flags & LOOKUP_PREFER_TYPES)
+	    && qualify_lookup (iter->type, flags))
+	  binding = iter->type;
+	else if (qualify_lookup (iter->value, flags))
 	  binding = iter->value;
-	else if ((flags & LOOKUP_PREFER_TYPES)
-		 && qualify_lookup (iter->type, flags))
-	  binding = iter->type;
 	else
 	  binding = NULL_TREE;
 
Index: testsuite/g++.dg/parse/typedef10.C
===================================================================
--- testsuite/g++.dg/parse/typedef10.C	(revision 0)
+++ testsuite/g++.dg/parse/typedef10.C	(working copy)
@@ -0,0 +1,20 @@
+// PR c++/46206
+
+class Foo1
+{
+  int u, v, w, x;
+  typedef struct Bar { } Bar;
+  virtual void foo(void) {
+    struct Bar bar;
+  }
+};
+
+class Foo2
+{
+  int u, v, w;
+  typedef struct Bar { } Bar;
+  Bar bar;
+  virtual void foo(void) {
+    struct Bar bar;
+  }
+};

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-06  9:26   ` Paolo Carlini
  2013-08-06 16:35     ` Paolo Carlini
@ 2013-08-06 22:09     ` Jason Merrill
  2013-08-06 22:14       ` Paolo Carlini
  1 sibling, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2013-08-06 22:09 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

On 08/06/2013 05:26 AM, Paolo Carlini wrote:
>> That's strange.  I would expect that to mean that we don't properly
>> give an error for a Bar data member declared after the typedef.

> You mean something like this?
>
> class Foo
> {
>    int u, v, w;//, x;
>    typedef struct Bar { } Bar;
>    Bar bar;
>    virtual void foo(void) {
>      struct Bar bar;
>    }
> };

I mean something like

class Foo
{
   int u, v, w;//, x;
   typedef struct Bar { } Bar;
   int Bar;
   virtual void foo(void) {
     struct Bar bar;
   }
};

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-06 22:09     ` Jason Merrill
@ 2013-08-06 22:14       ` Paolo Carlini
  2013-08-06 23:08         ` Jason Merrill
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Carlini @ 2013-08-06 22:14 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

Hi,

On 08/06/2013 11:19 PM, Jason Merrill wrote:
> I mean something like
>
> class Foo
> {
>   int u, v, w;//, x;
>   typedef struct Bar { } Bar;
>   int Bar;
>   virtual void foo(void) {
>     struct Bar bar;
>   }
> };
Ah I see, thanks. We reject this before and after the patch. Shall I add 
this variant to the new testcase?

Thanks,
Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-06 22:14       ` Paolo Carlini
@ 2013-08-06 23:08         ` Jason Merrill
  2013-08-07  8:42           ` Paolo Carlini
  0 siblings, 1 reply; 24+ messages in thread
From: Jason Merrill @ 2013-08-06 23:08 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: gcc-patches

On 08/06/2013 06:14 PM, Paolo Carlini wrote:
> Ah I see, thanks. We reject this before and after the patch. Shall I add
> this variant to the new testcase?

Sure.

Jason


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-06 23:08         ` Jason Merrill
@ 2013-08-07  8:42           ` Paolo Carlini
  0 siblings, 0 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-07  8:42 UTC (permalink / raw)
  To: Jason Merrill; +Cc: gcc-patches

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

Hi,

On 08/07/2013 01:08 AM, Jason Merrill wrote:
> On 08/06/2013 06:14 PM, Paolo Carlini wrote:
>> Ah I see, thanks. We reject this before and after the patch. Shall I add
>> this variant to the new testcase?
> Sure.
Thanks. Thus after a further round of testing I'm going to apply the below.

Thanks,
Paolo.

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

[-- Attachment #2: CL_46206 --]
[-- Type: text/plain, Size: 266 bytes --]

/cp
2013-08-07  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/46206
	* name-lookup.c (lookup_name_real_1): Handle iter->type before
	iter->value.

/testsuite
2013-08-07  Paolo Carlini  <paolo.carlini@oracle.com>

	PR c++/46206
	* g++.dg/lookup/typedef2.C: New.

[-- Attachment #3: patch_46206b --]
[-- Type: text/plain, Size: 1369 bytes --]

Index: cp/name-lookup.c
===================================================================
--- cp/name-lookup.c	(revision 201524)
+++ cp/name-lookup.c	(working copy)
@@ -4740,11 +4740,11 @@ lookup_name_real_1 (tree name, int prefer_type, in
 	  continue;
 
 	/* If this is the kind of thing we're looking for, we're done.  */
-	if (qualify_lookup (iter->value, flags))
+	if ((flags & LOOKUP_PREFER_TYPES)
+	    && qualify_lookup (iter->type, flags))
+	  binding = iter->type;
+	else if (qualify_lookup (iter->value, flags))
 	  binding = iter->value;
-	else if ((flags & LOOKUP_PREFER_TYPES)
-		 && qualify_lookup (iter->type, flags))
-	  binding = iter->type;
 	else
 	  binding = NULL_TREE;
 
Index: testsuite/g++.dg/lookup/typedef2.C
===================================================================
--- testsuite/g++.dg/lookup/typedef2.C	(revision 0)
+++ testsuite/g++.dg/lookup/typedef2.C	(working copy)
@@ -0,0 +1,30 @@
+// PR c++/46206
+
+class Foo1
+{
+  int u, v, w, x;
+  typedef struct Bar { } Bar;
+  virtual void foo(void) {
+    struct Bar bar;
+  }
+};
+
+class Foo2
+{
+  int u, v, w;
+  typedef struct Bar { } Bar;
+  Bar bar;
+  virtual void foo(void) {
+    struct Bar bar;
+  }
+};
+
+class Foo3
+{
+  int u, v, w;
+  typedef struct Bar { } Bar;
+  int Bar;   // { dg-error "conflicts" }
+  virtual void foo(void) {
+    struct Bar bar;
+  }
+};

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 15:47       ` Paolo Carlini
@ 2013-08-10 11:05         ` Iain Sandoe
  0 siblings, 0 replies; 24+ messages in thread
From: Iain Sandoe @ 2013-08-10 11:05 UTC (permalink / raw)
  To: Paolo Carlini
  Cc: David Edelsohn, Dominique Dhumieres, GCC Patches, Jason Merrill

Hi Paolo,

On 9 Aug 2013, at 16:47, Paolo Carlini wrote:
> On 08/09/2013 05:22 PM, David Edelsohn wrote:
>> Exactly. What is the common factor on AIX, Darwin and Solaris that is different from Linux? A difference in system types? How can we help? 
> Thanks David, all, for your kind offers.
> 
> As I said the issue is weird, I think the only way in practice to make progress is very serious debugging on AIX and either Darwin or Solaris. Note that AIX and Darwin are already different: on the latter only the second new subtest fails, that for Foo2, on AIX both.
> 
> For the time being I decided to revert the patch, otherwise the issue only makes me nervous. If somebody has insights, basing on my original analysis here maybe:
> 
>    http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00239.html
> 
> I would be glad to work again on the issue at a later time, in say a week or two. At the moment my TODO list is already full.

On Darwin, a couple more data points 
[current head, appears independent of build - i.e. stage1 built -O0 -g3 behaves the same as bootstrapped]

we accept when ints < 3,
we also accept when >= 4 (at least up to 15).
I.E. ACAICT, the specific case triggering this is num-int == 3.

If the name of the typedef is changed to Baz, we accept (maybe obvious, but checked anyway).

Trying to think of things that could cause this...
I wonder if this is some weird corner-case in name hashing?
Iain

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 15:22     ` David Edelsohn
@ 2013-08-09 15:47       ` Paolo Carlini
  2013-08-10 11:05         ` Iain Sandoe
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Carlini @ 2013-08-09 15:47 UTC (permalink / raw)
  To: David Edelsohn
  Cc: Iain Sandoe, Dominique Dhumieres, GCC Patches, Jason Merrill

Hi,

On 08/09/2013 05:22 PM, David Edelsohn wrote:
> Exactly. What is the common factor on AIX, Darwin and Solaris that is 
> different from Linux? A difference in system types? How can we help? 
Thanks David, all, for your kind offers.

As I said the issue is weird, I think the only way in practice to make 
progress is very serious debugging on AIX and either Darwin or Solaris. 
Note that AIX and Darwin are already different: on the latter only the 
second new subtest fails, that for Foo2, on AIX both.

For the time being I decided to revert the patch, otherwise the issue 
only makes me nervous. If somebody has insights, basing on my original 
analysis here maybe:

     http://gcc.gnu.org/ml/gcc-patches/2013-08/msg00239.html

I would be glad to work again on the issue at a later time, in say a 
week or two. At the moment my TODO list is already full.

Thanks again,
Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 11:35   ` Iain Sandoe
  2013-08-09 11:48     ` Rainer Orth
@ 2013-08-09 15:22     ` David Edelsohn
  2013-08-09 15:47       ` Paolo Carlini
  1 sibling, 1 reply; 24+ messages in thread
From: David Edelsohn @ 2013-08-09 15:22 UTC (permalink / raw)
  To: Iain Sandoe, Paolo Carlini
  Cc: Dominique Dhumieres, GCC Patches, Jason Merrill

On Fri, Aug 9, 2013 at 7:35 AM, Iain Sandoe <iain@codesourcery.com> wrote:
>
> On 9 Aug 2013, at 12:12, Paolo Carlini wrote:
>
>> On 08/09/2013 12:52 PM, dominiq@lps.ens.fr wrote:
>>> On x86_64-apple-darwin10, g++.dg/lookup/typedef2.C fails with
>>>
>>> FAIL: g++.dg/lookup/typedef2.C -std=c++11 (test for excess errors)
>>> Excess errors:
>>> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct'
>>> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token
>> As I said, the patch solves the issue on -linux, not just x86_64-linux, but Linux more generally. Honestly, I don't know how much it will take me to figure out a way to extend it to other targets, unless somebody concretely helps.
>
> what output/analysis would help you in the short term?
> (maybe looking at the common factors between more than one failing target will help to isolate the issue).

Exactly. What is the common factor on AIX, Darwin and Solaris that is
different from Linux? A difference in system types?

How can we help?

Thanks, David

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 11:48     ` Rainer Orth
@ 2013-08-09 12:06       ` Paolo Carlini
  0 siblings, 0 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-09 12:06 UTC (permalink / raw)
  To: Rainer Orth; +Cc: Iain Sandoe, dominiq, gcc-patches, jason, dje.gcc

.. reverted.

Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 11:35   ` Iain Sandoe
@ 2013-08-09 11:48     ` Rainer Orth
  2013-08-09 12:06       ` Paolo Carlini
  2013-08-09 15:22     ` David Edelsohn
  1 sibling, 1 reply; 24+ messages in thread
From: Rainer Orth @ 2013-08-09 11:48 UTC (permalink / raw)
  To: Iain Sandoe; +Cc: Paolo Carlini, dominiq, gcc-patches, jason, dje.gcc

Iain Sandoe <iain@codesourcery.com> writes:

> On 9 Aug 2013, at 12:12, Paolo Carlini wrote:
>
>> On 08/09/2013 12:52 PM, dominiq@lps.ens.fr wrote:
>>> On x86_64-apple-darwin10, g++.dg/lookup/typedef2.C fails with
>>> 
>>> FAIL: g++.dg/lookup/typedef2.C -std=c++11 (test for excess errors)
>>> Excess errors:
>>> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using
>>> typedef-name 'Foo1::Bar' after 'struct'
>>> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error:
>>> invalid type in declaration before ';' token
>> As I said, the patch solves the issue on -linux, not just x86_64-linux,
>> but Linux more generally. Honestly, I don't know how much it will take me
>> to figure out a way to extend it to other targets, unless somebody
>> concretely helps.
>
> what output/analysis would help you in the short term?
> (maybe looking at the common factors between more than one failing target
> will help to isolate the issue).

FWIW, I see a similar failure on i386-pc-solaris2.10:

FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
Excess errors:
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12: error: using typedef-name 'Foo2::Bar' after 'struct'
/vol/gcc/src/hg/trunk/local/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19: error: invalid type in declaration before ';' token

	Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 11:13 ` Paolo Carlini
@ 2013-08-09 11:35   ` Iain Sandoe
  2013-08-09 11:48     ` Rainer Orth
  2013-08-09 15:22     ` David Edelsohn
  0 siblings, 2 replies; 24+ messages in thread
From: Iain Sandoe @ 2013-08-09 11:35 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: dominiq, gcc-patches, jason, dje.gcc


On 9 Aug 2013, at 12:12, Paolo Carlini wrote:

> On 08/09/2013 12:52 PM, dominiq@lps.ens.fr wrote:
>> On x86_64-apple-darwin10, g++.dg/lookup/typedef2.C fails with
>> 
>> FAIL: g++.dg/lookup/typedef2.C -std=c++11 (test for excess errors)
>> Excess errors:
>> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct'
>> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token
> As I said, the patch solves the issue on -linux, not just x86_64-linux, but Linux more generally. Honestly, I don't know how much it will take me to figure out a way to extend it to other targets, unless somebody concretely helps.

what output/analysis would help you in the short term?
(maybe looking at the common factors between more than one failing target will help to isolate the issue).
Iain

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09 10:53 Dominique Dhumieres
@ 2013-08-09 11:13 ` Paolo Carlini
  2013-08-09 11:35   ` Iain Sandoe
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Carlini @ 2013-08-09 11:13 UTC (permalink / raw)
  To: dominiq; +Cc: gcc-patches, jason, dje.gcc

On 08/09/2013 12:52 PM, dominiq@lps.ens.fr wrote:
> On x86_64-apple-darwin10, g++.dg/lookup/typedef2.C fails with
>
> FAIL: g++.dg/lookup/typedef2.C -std=c++11 (test for excess errors)
> Excess errors:
> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct'
> /opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token
As I said, the patch solves the issue on -linux, not just x86_64-linux, 
but Linux more generally. Honestly, I don't know how much it will take 
me to figure out a way to extend it to other targets, unless somebody 
concretely helps.

If you want me to revert it, just say it, done.

Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
@ 2013-08-09 10:53 Dominique Dhumieres
  2013-08-09 11:13 ` Paolo Carlini
  0 siblings, 1 reply; 24+ messages in thread
From: Dominique Dhumieres @ 2013-08-09 10:53 UTC (permalink / raw)
  To: gcc-patches; +Cc: jason, dje.gcc, paolo.carlini

On x86_64-apple-darwin10, g++.dg/lookup/typedef2.C fails with

FAIL: g++.dg/lookup/typedef2.C -std=c++11 (test for excess errors)
Excess errors:
/opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12: error: using typedef-name 'Foo1::Bar' after 'struct'
/opt/gcc/work/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19: error: invalid type in declaration before ';' token

Dominique

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09  0:24       ` Jason Merrill
  2013-08-09  1:25         ` David Edelsohn
@ 2013-08-09  6:49         ` Paolo Carlini
  1 sibling, 0 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-09  6:49 UTC (permalink / raw)
  To: Jason Merrill, David Edelsohn; +Cc: GCC Patches



Hi

Jason Merrill <jason@redhat.com> ha scritto:

>Probably the same reason that subtle changes in the testcase changed
>whether the bug appeared on x86_64-linux.  I guess we should figure
>that
>out instead of just saying "hunh, that's odd."

For sure. Let's see if I can in a reasonable amount of time on x86_64-linux. At some point when I saw that comment in the code I thought the reason was already obvious to somebody, not to me but to somebody.

Paolo

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-09  0:24       ` Jason Merrill
@ 2013-08-09  1:25         ` David Edelsohn
  2013-08-09  6:49         ` Paolo Carlini
  1 sibling, 0 replies; 24+ messages in thread
From: David Edelsohn @ 2013-08-09  1:25 UTC (permalink / raw)
  To: Jason Merrill; +Cc: Paolo Carlini, GCC Patches

On Thu, Aug 8, 2013 at 8:24 PM, Jason Merrill <jason@redhat.com> wrote:
> On 08/08/2013 02:31 PM, Paolo Carlini wrote:
>>
>> On 08/08/2013 08:18 PM, David Edelsohn wrote:
>>>
>>> Why does the patch and fix have any architecture or OS-dependency?
>
>
> Probably the same reason that subtle changes in the testcase changed whether
> the bug appeared on x86_64-linux.  I guess we should figure that out instead
> of just saying "hunh, that's odd."

There's an AIX system in the GCC Compile Farm and I also can provide
additional dump file output if you tell me what would be helpful.

Thanks, David

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-08 18:31     ` Paolo Carlini
  2013-08-08 19:28       ` Paolo Carlini
@ 2013-08-09  0:24       ` Jason Merrill
  2013-08-09  1:25         ` David Edelsohn
  2013-08-09  6:49         ` Paolo Carlini
  1 sibling, 2 replies; 24+ messages in thread
From: Jason Merrill @ 2013-08-09  0:24 UTC (permalink / raw)
  To: Paolo Carlini, David Edelsohn; +Cc: GCC Patches

On 08/08/2013 02:31 PM, Paolo Carlini wrote:
> On 08/08/2013 08:18 PM, David Edelsohn wrote:
>> Why does the patch and fix have any architecture or OS-dependency?

Probably the same reason that subtle changes in the testcase changed 
whether the bug appeared on x86_64-linux.  I guess we should figure that 
out instead of just saying "hunh, that's odd."

Jason


^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-08 18:31     ` Paolo Carlini
@ 2013-08-08 19:28       ` Paolo Carlini
  2013-08-09  0:24       ` Jason Merrill
  1 sibling, 0 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-08 19:28 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: David Edelsohn, GCC Patches, Jason Merrill

On 08/08/2013 08:31 PM, Paolo Carlini wrote:
> On 08/08/2013 08:18 PM, David Edelsohn wrote:
>> Why does the patch and fix have any architecture or OS-dependency?
> It's tricky, but it depends on the way are internally stored and 
> handled *multiple* TYPE_DECL for essentially the same typedef. The 
> class layout must involve both such typedef and a few data members.
... and at least a virtual function, forgot this important detail. I 
don't think you can construct a reject-valid of this type on Linux 
without a virtual function.

Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-08 18:18   ` David Edelsohn
@ 2013-08-08 18:31     ` Paolo Carlini
  2013-08-08 19:28       ` Paolo Carlini
  2013-08-09  0:24       ` Jason Merrill
  0 siblings, 2 replies; 24+ messages in thread
From: Paolo Carlini @ 2013-08-08 18:31 UTC (permalink / raw)
  To: David Edelsohn; +Cc: GCC Patches, Jason Merrill

On 08/08/2013 08:18 PM, David Edelsohn wrote:
> Why does the patch and fix have any architecture or OS-dependency?
It's tricky, but it depends on the way are internally stored and handled 
*multiple* TYPE_DECL for essentially the same typedef. The class layout 
must involve both such typedef and a few data members. I had a look to 
testresults, and the patch appears to work for other -linux targets too, 
not just x86_64-linux, and it's elementary, doesn't add complexity to 
the code, thus unless it proves to cause regressions on the targets 
where it appears to work, it would be a pity to revert it. But as I said 
the issue is weird, after all remained unfixed for 3-4 years, 
realistically, I don't know if together with all the other issues I have 
in my TODO, I can promise to understand it in such detail to fix it on 
AIX too.

Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-08 17:57 ` Paolo Carlini
@ 2013-08-08 18:18   ` David Edelsohn
  2013-08-08 18:31     ` Paolo Carlini
  0 siblings, 1 reply; 24+ messages in thread
From: David Edelsohn @ 2013-08-08 18:18 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: GCC Patches, Jason Merrill

Why does the patch and fix have any architecture or OS-dependency?

- David

On Thu, Aug 8, 2013 at 1:57 PM, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> On 08/08/2013 07:35 PM, David Edelsohn wrote:
>>
>> I'm now seeing a new testsuite failure:
>>
>> FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
>> Excess errors:
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12:
>> error: using typedef-name 'Foo1::Bar' after 'struct'
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19:
>> error: invalid type in declaration before ';' token
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12:
>> error: using typedef-name 'Foo2::Bar' after 'struct'
>> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19:
>> error: invalid type in declaration before ';' token
>
> Which essentially means that my patchlet fixes the problem on x86_64-linux,
> but not on AIX. Because you would see the exact same fails if I revert the
> patch and keep the testcase in.
>
> I'm ready to revert the patch and re-opening or xfail-ing on AIX. Jason call
> I guess.
>
> Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
  2013-08-08 17:35 David Edelsohn
@ 2013-08-08 17:57 ` Paolo Carlini
  2013-08-08 18:18   ` David Edelsohn
  0 siblings, 1 reply; 24+ messages in thread
From: Paolo Carlini @ 2013-08-08 17:57 UTC (permalink / raw)
  To: David Edelsohn; +Cc: GCC Patches, Jason Merrill

On 08/08/2013 07:35 PM, David Edelsohn wrote:
> I'm now seeing a new testsuite failure:
>
> FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
> Excess errors:
> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12:
> error: using typedef-name 'Foo1::Bar' after 'struct'
> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19:
> error: invalid type in declaration before ';' token
> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12:
> error: using typedef-name 'Foo2::Bar' after 'struct'
> /nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19:
> error: invalid type in declaration before ';' token
Which essentially means that my patchlet fixes the problem on 
x86_64-linux, but not on AIX. Because you would see the exact same fails 
if I revert the patch and keep the testcase in.

I'm ready to revert the patch and re-opening or xfail-ing on AIX. Jason 
call I guess.

Paolo.

^ permalink raw reply	[flat|nested] 24+ messages in thread

* Re: [C++ Patch / RFC] PR 46206
@ 2013-08-08 17:35 David Edelsohn
  2013-08-08 17:57 ` Paolo Carlini
  0 siblings, 1 reply; 24+ messages in thread
From: David Edelsohn @ 2013-08-08 17:35 UTC (permalink / raw)
  To: Paolo Carlini; +Cc: GCC Patches, Jason Merrill

I'm now seeing a new testsuite failure:

FAIL: g++.dg/lookup/typedef2.C -std=c++98 (test for excess errors)
Excess errors:
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:12:
error: using typedef-name 'Foo1::Bar' after 'struct'
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:8:19:
error: invalid type in declaration before ';' token
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:12:
error: using typedef-name 'Foo2::Bar' after 'struct'
/nasfarm/edelsohn/src/src/gcc/testsuite/g++.dg/lookup/typedef2.C:18:19:
error: invalid type in declaration before ';' token

Thanks, David

^ permalink raw reply	[flat|nested] 24+ messages in thread

end of thread, other threads:[~2013-08-10 11:05 UTC | newest]

Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2013-08-05 22:46 [C++ Patch / RFC] PR 46206 Paolo Carlini
2013-08-06  2:57 ` Jason Merrill
2013-08-06  9:26   ` Paolo Carlini
2013-08-06 16:35     ` Paolo Carlini
2013-08-06 22:09     ` Jason Merrill
2013-08-06 22:14       ` Paolo Carlini
2013-08-06 23:08         ` Jason Merrill
2013-08-07  8:42           ` Paolo Carlini
2013-08-08 17:35 David Edelsohn
2013-08-08 17:57 ` Paolo Carlini
2013-08-08 18:18   ` David Edelsohn
2013-08-08 18:31     ` Paolo Carlini
2013-08-08 19:28       ` Paolo Carlini
2013-08-09  0:24       ` Jason Merrill
2013-08-09  1:25         ` David Edelsohn
2013-08-09  6:49         ` Paolo Carlini
2013-08-09 10:53 Dominique Dhumieres
2013-08-09 11:13 ` Paolo Carlini
2013-08-09 11:35   ` Iain Sandoe
2013-08-09 11:48     ` Rainer Orth
2013-08-09 12:06       ` Paolo Carlini
2013-08-09 15:22     ` David Edelsohn
2013-08-09 15:47       ` Paolo Carlini
2013-08-10 11:05         ` Iain Sandoe

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).