* [C++ Patch] PR 44625
@ 2011-06-23 11:56 Paolo Carlini
2011-06-23 15:17 ` Jason Merrill
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 11:56 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 133 bytes --]
Hi,
a patchlet for a pretty old ice-on-invalid regression. Tested x86_64-linux.
Ok for mainline?
Thanks,
Paolo.
////////////////
[-- Attachment #2: CL_44625 --]
[-- Type: text/plain, Size: 261 bytes --]
/cp
2011-06-23 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/44625
* pt.c (tsubst_copy_and_build): Do not use BASELINK_P on a NULL_TREE.
/testsuite
2011-06-23 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/44625
* g++.dg/template/crash107.C: New.
[-- Attachment #3: patch_44625 --]
[-- Type: text/plain, Size: 1360 bytes --]
Index: testsuite/g++.dg/template/crash107.C
===================================================================
--- testsuite/g++.dg/template/crash107.C (revision 0)
+++ testsuite/g++.dg/template/crash107.C (revision 0)
@@ -0,0 +1,20 @@
+// PR c++/44625
+// { dg-do compile }
+// { dg-options "" }
+
+template<typename FP_> struct Vec { // { dg-message "note" }
+ Vec& operator^=(Vec& rhs) {
+ union {
+ struct {FP_ x,y,z;};
+ }; // { dg-error "anonymous struct" }
+ X = y*rhs.z() - z*rhs.y(); // { dg-error "not declared|no member" }
+ }
+ Vec& operator^(Vec& rhs) {
+ return Vec(*this)^=rhs; // { dg-message "required" }
+ }
+};
+Vec<double> v(3,4,12); // { dg-error "no matching" }
+// { dg-message "note" { target *-*-* } 16 }
+Vec<double> V(12,4,3); // { dg-error "no matching" }
+// { dg-message "note" { target *-*-* } 18 }
+Vec<double> c = v^V; // { dg-message "required" }
Index: cp/pt.c
===================================================================
--- cp/pt.c (revision 175328)
+++ cp/pt.c (working copy)
@@ -13252,6 +13252,9 @@ tsubst_copy_and_build (tree t,
object_type = TREE_TYPE (object);
member = TREE_OPERAND (t, 1);
+ if (!member)
+ return error_mark_node;
+
if (BASELINK_P (member))
member = tsubst_baselink (member,
non_reference (TREE_TYPE (object)),
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 11:56 [C++ Patch] PR 44625 Paolo Carlini
@ 2011-06-23 15:17 ` Jason Merrill
2011-06-23 15:47 ` Paolo Carlini
0 siblings, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2011-06-23 15:17 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
Why are we creating a COMPONENT_REF with a null op1 in the first place?
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 15:17 ` Jason Merrill
@ 2011-06-23 15:47 ` Paolo Carlini
2011-06-23 16:01 ` Jason Merrill
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 15:47 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
Hi,
> Why are we creating a COMPONENT_REF with a null op1 in the first place?
For now, what I figured out is the following: build_anon_union_vars
calls build_min_nt (COMPONENT_REF, object, DECL_NAME (field), NULL_TREE)
with a null third argument, which becomes the null op1.
As a matter of fact, the possibility that DECL_NAME (field) could be
null is considered in build_anon_union_vars itself, because right after
the above mentioned build_min_nt call, there is a conditional 'if
(DECL_NAME (field)) ...'
I don't know if the above is enough for you to suggest the way we should
go...
Paolo.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 15:47 ` Paolo Carlini
@ 2011-06-23 16:01 ` Jason Merrill
2011-06-23 16:06 ` Paolo Carlini
0 siblings, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2011-06-23 16:01 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
On 06/23/2011 11:30 AM, Paolo Carlini wrote:
>> Why are we creating a COMPONENT_REF with a null op1 in the first place?
> As a matter of fact, the possibility that DECL_NAME (field) could be
> null is considered in build_anon_union_vars itself, because right after
> the above mentioned build_min_nt call, there is a conditional 'if
> (DECL_NAME (field)) ...'
Indeed. The code is using DECL_NAME in templates so that tsubst can
look them up again by name, but as we see in this PR that can't work if
the field has no name. We need a different strategy for handling
anonymous aggregates nested in other anonymous aggregates in templates.
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:06 ` Jason Merrill
@ 2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:13 ` Paolo Carlini
2011-06-23 16:53 ` Paolo Carlini
2011-06-23 16:11 ` Paolo Carlini
1 sibling, 2 replies; 15+ messages in thread
From: Jason Merrill @ 2011-06-23 16:06 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
Actually, 9.5 says
A union of the form
union { member-specification } ;
is called an anonymous union; it defines an unnamed object of unnamed
type. The member-specification of an anonymous union shall only define
non-static data members. [ Note: Nested types and functions cannot
be declared within an anonymous union. — end note ]
So we should be able to just reject nested anonymous aggregates and not
worry about how to make them work.
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:01 ` Jason Merrill
@ 2011-06-23 16:06 ` Paolo Carlini
2011-06-23 16:06 ` Jason Merrill
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 16:06 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On 06/23/2011 05:47 PM, Jason Merrill wrote:
> Indeed. The code is using DECL_NAME in templates so that tsubst can
> look them up again by name, but as we see in this PR that can't work
> if the field has no name. We need a different strategy for handling
> anonymous aggregates nested in other anonymous aggregates in templates.
Ok, then, from what you saying I understand that it should be possible
to actually construct a reject-valid or an ice-on-valid in this area,
isn't just about improving the diagnostic, that is only the tip of the
iceberg, so to speak. I guess it's not for me, at this time...
Paolo.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:06 ` Paolo Carlini
@ 2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:11 ` Paolo Carlini
0 siblings, 2 replies; 15+ messages in thread
From: Jason Merrill @ 2011-06-23 16:06 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
On 06/23/2011 11:52 AM, Paolo Carlini wrote:
> Ok, then, from what you saying I understand that it should be possible
> to actually construct a reject-valid or an ice-on-valid in this area,
> isn't just about improving the diagnostic, that is only the tip of the
> iceberg, so to speak. I guess it's not for me, at this time...
For the time being you could improve the diagnostic by adding a sorry
for the case of null DECL_NAME in a template in build_anon_union_vars.
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:06 ` Jason Merrill
@ 2011-06-23 16:11 ` Paolo Carlini
1 sibling, 0 replies; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 16:11 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On 06/23/2011 06:01 PM, Jason Merrill wrote:
> On 06/23/2011 11:52 AM, Paolo Carlini wrote:
>> Ok, then, from what you saying I understand that it should be possible
>> to actually construct a reject-valid or an ice-on-valid in this area,
>> isn't just about improving the diagnostic, that is only the tip of the
>> iceberg, so to speak. I guess it's not for me, at this time...
> For the time being you could improve the diagnostic by adding a sorry
> for the case of null DECL_NAME in a template in build_anon_union_vars.
Indeed. Let me try then...
Paolo.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:06 ` Jason Merrill
@ 2011-06-23 16:13 ` Paolo Carlini
2011-06-23 16:19 ` Paolo Carlini
2011-06-23 16:53 ` Paolo Carlini
1 sibling, 1 reply; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 16:13 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On 06/23/2011 06:05 PM, Jason Merrill wrote:
> Actually, 9.5 says
>
> A union of the form
> union { member-specification } ;
> is called an anonymous union; it defines an unnamed object of unnamed
> type. The member-specification of an anonymous union shall only define
> non-static data members. [ Note: Nested types and functions cannot
> be declared within an anonymous union. — end note ]
>
> So we should be able to just reject nested anonymous aggregates and
> not worry about how to make them work.
Yes, but we are accepting already some of that as an extension. If I
compile the testcase with -pedantic-errors I get:
44625.C:8:31: error: ISO C++ prohibits anonymous structs [-pedantic]
44625.C:9:9: error: anonymous struct not inside named type
thus, it's not clear to me where we should stop, exactly.
Paolo.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:13 ` Paolo Carlini
@ 2011-06-23 16:19 ` Paolo Carlini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 16:19 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On 06/23/2011 06:11 PM, Paolo Carlini wrote:
> On 06/23/2011 06:05 PM, Jason Merrill wrote:
>> Actually, 9.5 says
>>
>> A union of the form
>> union { member-specification } ;
>> is called an anonymous union; it defines an unnamed object of unnamed
>> type. The member-specification of an anonymous union shall only
>> define non-static data members. [ Note: Nested types and functions
>> cannot
>> be declared within an anonymous union. — end note ]
>>
>> So we should be able to just reject nested anonymous aggregates and
>> not worry about how to make them work.
> Yes, but we are accepting already some of that as an extension. If I
> compile the testcase with -pedantic-errors I get:
>
> 44625.C:8:31: error: ISO C++ prohibits anonymous structs [-pedantic]
> 44625.C:9:9: error: anonymous struct not inside named type
Of course only the first message is new with -pedantic-errors.
Thus, the idea would be rejecting *nested* anonymous, now I see. Uhmm.
Paolo.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:13 ` Paolo Carlini
@ 2011-06-23 16:53 ` Paolo Carlini
2011-06-23 16:54 ` Jason Merrill
1 sibling, 1 reply; 15+ messages in thread
From: Paolo Carlini @ 2011-06-23 16:53 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
[-- Attachment #1: Type: text/plain, Size: 438 bytes --]
On 06/23/2011 06:05 PM, Jason Merrill wrote:
> So we should be able to just reject nested anonymous aggregates and
> not worry about how to make them work.
The below appears to work pretty well, regtests fine. I had to tweak the
existing error17.C, we don't emit anymore the warning about no members
in the nested anonymous struct, because we bail out early. Doesn't seem
a serious issue to me...
Ok?
Paolo.
//////////////////////
[-- Attachment #2: CL_44625_2 --]
[-- Type: text/plain, Size: 325 bytes --]
/cp
2011-06-23 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/44625
* decl2.c (build_anon_union_vars): Early return error_mark_node
for a nested anonymous struct.
/testsuite
2011-06-23 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/44625
* g++.dg/template/crash107.C: New.
* g++.dg/template/error17.C: Adjust.
[-- Attachment #3: patch_44625_2 --]
[-- Type: text/plain, Size: 1974 bytes --]
Index: testsuite/g++.dg/template/error17.C
===================================================================
--- testsuite/g++.dg/template/error17.C (revision 175330)
+++ testsuite/g++.dg/template/error17.C (working copy)
@@ -6,5 +6,4 @@ foo()
{
union { struct { }; }; // { dg-error "prohibits anonymous struct" "anon" }
// { dg-error "not inside" "not inside" { target *-*-* } 7 }
- // { dg-warning "no members" "no members" { target *-*-* } 7 }
}
Index: testsuite/g++.dg/template/crash107.C
===================================================================
--- testsuite/g++.dg/template/crash107.C (revision 0)
+++ testsuite/g++.dg/template/crash107.C (revision 0)
@@ -0,0 +1,20 @@
+// PR c++/44625
+// { dg-do compile }
+// { dg-options "" }
+
+template<typename FP_> struct Vec { // { dg-message "note" }
+ Vec& operator^=(Vec& rhs) {
+ union {
+ struct {FP_ x,y,z;};
+ }; // { dg-error "anonymous struct" }
+ X = y*rhs.z() - z*rhs.y(); // { dg-error "not declared|no member" }
+ }
+ Vec& operator^(Vec& rhs) {
+ return Vec(*this)^=rhs; // { dg-message "required" }
+ }
+};
+Vec<double> v(3,4,12); // { dg-error "no matching" }
+// { dg-message "note" { target *-*-* } 16 }
+Vec<double> V(12,4,3); // { dg-error "no matching" }
+// { dg-message "note" { target *-*-* } 18 }
+Vec<double> c = v^V; // { dg-message "required" }
Index: cp/decl2.c
===================================================================
--- cp/decl2.c (revision 175335)
+++ cp/decl2.c (working copy)
@@ -1327,7 +1327,10 @@ build_anon_union_vars (tree type, tree object)
/* Rather than write the code to handle the non-union case,
just give an error. */
if (TREE_CODE (type) != UNION_TYPE)
- error ("anonymous struct not inside named type");
+ {
+ error ("anonymous struct not inside named type");
+ return error_mark_node;
+ }
for (field = TYPE_FIELDS (type);
field != NULL_TREE;
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2011-06-23 16:53 ` Paolo Carlini
@ 2011-06-23 16:54 ` Jason Merrill
0 siblings, 0 replies; 15+ messages in thread
From: Jason Merrill @ 2011-06-23 16:54 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
OK.
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2010-07-05 13:10 ` Jason Merrill
@ 2010-07-05 13:13 ` Paolo Carlini
0 siblings, 0 replies; 15+ messages in thread
From: Paolo Carlini @ 2010-07-05 13:13 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches
On 07/05/2010 03:10 PM, Jason Merrill wrote:
> On 07/05/2010 07:52 AM, Paolo Carlini wrote:
>> I have this patchlet for another case where we use BASELINK_P on
>> NULL_TREE. Tested x86_64-linux. I propose to apply it to mainline and
>> 4_5-branch only and then close the PR: it's a regression, but just a P5
>> ice-on-invalid.
> Seems like we ought to catch this when we're creating the
> COMPONENT_REF, rather than when we're trying to substitute into it; we
> shouldn't create a COMPONENT_REF with a null operand 1.
Ok, I'll look into it.
Paolo.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [C++ Patch] PR 44625
2010-07-05 11:52 Paolo Carlini
@ 2010-07-05 13:10 ` Jason Merrill
2010-07-05 13:13 ` Paolo Carlini
0 siblings, 1 reply; 15+ messages in thread
From: Jason Merrill @ 2010-07-05 13:10 UTC (permalink / raw)
To: Paolo Carlini; +Cc: gcc-patches
On 07/05/2010 07:52 AM, Paolo Carlini wrote:
> I have this patchlet for another case where we use BASELINK_P on
> NULL_TREE. Tested x86_64-linux. I propose to apply it to mainline and
> 4_5-branch only and then close the PR: it's a regression, but just a P5
> ice-on-invalid.
Seems like we ought to catch this when we're creating the COMPONENT_REF,
rather than when we're trying to substitute into it; we shouldn't create
a COMPONENT_REF with a null operand 1.
Jason
^ permalink raw reply [flat|nested] 15+ messages in thread
* [C++ Patch] PR 44625
@ 2010-07-05 11:52 Paolo Carlini
2010-07-05 13:10 ` Jason Merrill
0 siblings, 1 reply; 15+ messages in thread
From: Paolo Carlini @ 2010-07-05 11:52 UTC (permalink / raw)
To: gcc-patches; +Cc: Jason Merrill
[-- Attachment #1: Type: text/plain, Size: 264 bytes --]
Hi,
I have this patchlet for another case where we use BASELINK_P on
NULL_TREE. Tested x86_64-linux. I propose to apply it to mainline and
4_5-branch only and then close the PR: it's a regression, but just a P5
ice-on-invalid.
Thanks,
Paolo.
//////////////////
[-- Attachment #2: CL_44625 --]
[-- Type: text/plain, Size: 262 bytes --]
/cp
2010-07-05 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/44625
* pt.c (tsubst_copy_and_build): Do not apply BASELINK_P on
NULL_TREE.
/testsuite
2010-07-05 Paolo Carlini <paolo.carlini@oracle.com>
PR c++/44625
* g++.dg/template/crash102.C: New.
[-- Attachment #3: patch_44625 --]
[-- Type: text/plain, Size: 1558 bytes --]
Index: testsuite/g++.dg/template/crash102.C
===================================================================
--- testsuite/g++.dg/template/crash102.C (revision 0)
+++ testsuite/g++.dg/template/crash102.C (revision 0)
@@ -0,0 +1,19 @@
+// PR c++/44625
+// { dg-do compile }
+// { dg-options "" }
+
+template<typename FP_> struct Vec { // { dg-message "note" }
+ Vec& operator^=(Vec& rhs) {
+ union {
+ struct {FP_ x,y,z;};
+ }; // { dg-error "anonymous struct" }
+ y*rhs.z() - z*rhs.y(); // { dg-error "no member" }
+ return *this;
+ }
+ Vec& operator^(Vec& rhs) {
+ return Vec(*this)^=rhs; // { dg-message "instantiated" }
+ }
+};
+Vec<double> v(3,4,12); // { dg-error "no matching" }
+Vec<double> V(12,4,3); // { dg-error "no matching" }
+Vec<double> c = v^V; // { dg-message "instantiated" }
Index: cp/pt.c
===================================================================
--- cp/pt.c (revision 161825)
+++ cp/pt.c (working copy)
@@ -12625,7 +12625,9 @@ tsubst_copy_and_build (tree t,
{
tree object;
tree object_type;
- tree member;
+ tree member = TREE_OPERAND (t, 1);
+ if (!member)
+ return error_mark_node;
object = tsubst_non_call_postfix_expression (TREE_OPERAND (t, 0),
args, complain, in_decl);
@@ -12634,7 +12636,6 @@ tsubst_copy_and_build (tree t,
mark_used (object);
object_type = TREE_TYPE (object);
- member = TREE_OPERAND (t, 1);
if (BASELINK_P (member))
member = tsubst_baselink (member,
non_reference (TREE_TYPE (object)),
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-06-23 16:53 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-06-23 11:56 [C++ Patch] PR 44625 Paolo Carlini
2011-06-23 15:17 ` Jason Merrill
2011-06-23 15:47 ` Paolo Carlini
2011-06-23 16:01 ` Jason Merrill
2011-06-23 16:06 ` Paolo Carlini
2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:06 ` Jason Merrill
2011-06-23 16:13 ` Paolo Carlini
2011-06-23 16:19 ` Paolo Carlini
2011-06-23 16:53 ` Paolo Carlini
2011-06-23 16:54 ` Jason Merrill
2011-06-23 16:11 ` Paolo Carlini
-- strict thread matches above, loose matches on Subject: below --
2010-07-05 11:52 Paolo Carlini
2010-07-05 13:10 ` Jason Merrill
2010-07-05 13:13 ` Paolo Carlini
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).