* [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).