[Bug c++/20280] New: internal compiler error: in create_tmp_var, at gimplify.c:368

[Bug c++/20280] New: internal compiler error: in create_tmp_var, at gimplify.c:368

[caolanm at redhat dot com]

gcc (GCC) 4.0.0 20050228 (Red Hat 4.0.0-0.30)
Copyright (C) 2005 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.

---

Making: ../../../unxlngi6.pro/slo/fltini.obj
g++ -Wreturn-type -fmessage-length=0 -c -I.  -I. -I../inc -I../../../inc
-I../../../unx/inc -I../../../unxlngi6.pro/inc -I.
-I/usr/src/redhat/BUILD/SRC680_m80/solver/680/unxlngi6.pro/inc/stl
-I/usr/src/redhat/BUILD/SRC680_m80/solver/680/unxlngi6.pro/inc/external
-I/usr/src/redhat/BUILD/SRC680_m80/solver/680/unxlngi6.pro/inc
-I/usr/src/redhat/BUILD/SRC680_m80/solenv/unxlngi6/inc
-I/usr/src/redhat/BUILD/SRC680_m80/solenv/inc
-I/usr/src/redhat/BUILD/SRC680_m80/res
-I/usr/src/redhat/BUILD/SRC680_m80/solver/680/unxlngi6.pro/inc/stl
-I/usr/src/redhat/BUILD/SRC680_m80/solenv/inc/Xp31 -I/usr/include
-I/usr/X11R6/include     -I. -I../../../res -I. -Wuninitialized -g1 -Os
-fno-strict-aliasing   -fvisibility=hidden -pipe -mtune=pentiumpro
-Wno-ctor-dtor-privacy -fvisibility-inlines-hidden -fexceptions
-fno-enforce-eh-specs   -fpic -DLINUX -DUNX -DVCL -DGCC -DC341 -DINTEL
-DGXX_INCLUDE_PATH=/usr/lib/gcc/i386-redhat-linux/4.0.0/../../../../include/c++/4.0.0
-DCVER=C341 -D_USE_NAMESPACE -DGLIBC=2 -DX86 -D_PTHREADS -D_REENTRANT
-DNEW_SOLAR -D_USE_NAMESPACE=1 -DSTLPORT_VERSION=400
-DHAVE_GCC_VISIBILITY_FEATURE -D__DMAKE -DUNIX -DCPPU_ENV=gcc3 -DSUPD=680
-DPRODUCT -DNDEBUG -DPRODUCT_FULL -DOSL_DEBUG_LEVEL=0 -DOPTIMIZE -DEXCEPTIONS_ON
-DCUI -DSOLAR_JAVA -DSRC680   -DNUM_RELSPACE -DVERTICAL_LAYOUT
-DACCESSIBLE_LAYOUT -DBIDI -DSW_DLLIMPLEMENTATION -DSHAREDLIB -D_DLL_ 
-DMULTITHREAD  -o ../../../unxlngi6.pro/slo/fltini.o
/usr/src/redhat/BUILD/SRC680_m80/sw/source/filter/basflt/fltini.cxx
/usr/src/redhat/BUILD/SRC680_m80/sw/source/filter/basflt/fltini.cxx: In function
\uffff\uffff\uffffvoid CalculateFlySize(SfxItemSet&, const SwNodeIndex&,
SwTwips)\uffff\uffff\uffff:
/usr/src/redhat/BUILD/SRC680_m80/sw/source/filter/basflt/fltini.cxx:810:
internal compiler error: in create_tmp_var, at gimplify.c:368
Please submit a full bug report,
with preprocessed source if appropriate.
See <URL:http://bugzilla.redhat.com/bugzilla> for instructions.
Preprocessed source stored into /tmp/ccAHLsI1.out file, please attach this to
your bugreport.
dmake:  Error code 1, while making '../../../unxlngi6.pro/slo/fltini.obj'
'---* tg_merge.mk *---'
dmake:  Error code 255, while making 'do_it_exceptions'
'---* tg_merge.mk *---'

-- 
           Summary: internal compiler error: in create_tmp_var, at
                    gimplify.c:368
           Product: gcc
           Version: 4.0.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P2
         Component: c++
        AssignedTo: unassigned at gcc dot gnu dot org
        ReportedBy: caolanm at redhat dot com
                CC: gcc-bugs at gcc dot gnu dot org,jakub at gcc dot gnu dot
                    org


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] internal compiler error: in create_tmp_var, at gimplify.c:368

[caolanm at redhat dot com]

------- Additional Comments From caolanm at redhat dot com  2005-03-02 09:39 -------
Created an attachment (id=8311)
 --> (http://gcc.gnu.org/bugzilla/attachment.cgi?id=8311&action=view)
bzip2'ed dump


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[reichelt at gcc dot gnu dot org]

------- Additional Comments From reichelt at gcc dot gnu dot org  2005-03-02 13:31 -------
Confirmed. Reduced testcase:

==================================
struct A
{
    ~A();
};

struct B : A {};

A& foo();

void bar(bool b)
{
    (B&) (b ? foo() : foo());
}
==================================


-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |reichelt at gcc dot gnu dot
                   |                            |org
           Severity|normal                      |critical
             Status|UNCONFIRMED                 |NEW
     Ever Confirmed|                            |1
           Keywords|                            |ice-on-valid-code, monitored
   Last reconfirmed|0000-00-00 00:00:00         |2005-03-02 13:31:27
               date|                            |
            Summary|internal compiler error: in |[4.0/4.1 regression] ICE in
                   |create_tmp_var, at          |create_tmp_var, at
                   |gimplify.c:368              |gimplify.c:368
   Target Milestone|---                         |4.0.0


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at gcc dot gnu dot org]

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
         AssignedTo|unassigned at gcc dot gnu   |aoliva at gcc dot gnu dot
                   |dot org                     |org
             Status|NEW                         |ASSIGNED
   Last reconfirmed|2005-03-02 13:31:27         |2005-03-03 07:01:44
               date|                            |


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at redhat dot com]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-03 07:51 -------
Subject: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

When passing an lvalue cond_expr to a function taking a reference that
binds directly to either operand of ?:, we'd fail gimplification when
the type of the expressions was addressable, because, once again,
create_tmp_var would reject addressable types.

The solution I came up with was to hoist the indirect_ref out of the
cond_expr at the time it was created, such that we create a tmp_var
with a reference to the result of the cond_expr, and when we take the
address of the cond_expr to pass its result by reference, it cancels
out with the indirect_ref, so it works beautifully.

I went ahead and verified that I didn't break bit-field lvalues in
conditional expressions (my first attempt did), but I was surprised to
find out that the calls to h() pass.  I understand why they do (we
create a temporary and bind to that), but I'm not sure this is correct
behavior.  Opinions?

I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
PR c++/20103; it's also passed C++ regression testing.  Ok to install
if bootstrap and all-languages regression testing passes?

Index: gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20280
	* call.c (build_conditional_expr): Hoist indirect_ref out of
	cond_expr if the result is an addressable lvalue.

Index: gcc/cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 -0000 1.531
+++ gcc/cp/call.c 3 Mar 2005 07:42:24 -0000
@@ -3111,6 +3111,7 @@ build_conditional_expr (tree arg1, tree 
   tree result = NULL_TREE;
   tree result_type = NULL_TREE;
   bool lvalue_p = true;
+  bool indirect_p = false;
   struct z_candidate *candidates = 0;
   struct z_candidate *cand;
   void *p;
@@ -3292,7 +3293,15 @@ build_conditional_expr (tree arg1, tree 
       && real_lvalue_p (arg3) 
       && same_type_p (arg2_type, arg3_type))
     {
-      result_type = arg2_type;
+      if (TREE_ADDRESSABLE (arg2_type) && TREE_ADDRESSABLE (arg3_type))
+	{
+	  indirect_p = true;
+	  result_type = build_pointer_type (arg2_type);
+	  arg2 = fold_if_not_in_template (build_address (arg2));
+	  arg3 = fold_if_not_in_template (build_address (arg3));
+	}
+      else
+	result_type = arg2_type;
       goto valid_operands;
     }
 
@@ -3458,6 +3467,14 @@ build_conditional_expr (tree arg1, tree 
   /* We can't use result_type below, as fold might have returned a
      throw_expr.  */
 
+  if (indirect_p)
+    {
+      if (TREE_CODE (TREE_TYPE (result)) == POINTER_TYPE)
+	result = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (result)), result);
+      else
+	gcc_assert (TREE_CODE (result) == THROW_EXPR);
+    }
+
   /* Expand both sides into the same slot, hopefully the target of the
      ?: expression.  We used to check for TARGET_EXPRs here, but now we
      sometimes wrap them in NOP_EXPRs so the test would fail.  */
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* g++.dg/tree-ssa/pr20103.C: New.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 3 Mar 2005 07:42:38 -0000
@@ -0,0 +1,62 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+    ~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+    (B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <aoliva@redhat.com>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+  long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+  (b ? x.i : x.j) = 1;
+  (b ? x.j : x.k) = 2;
+  (b ? x.i : x.k) = 3;
+
+  (void)(b ? x.i : x.j);
+  (void)(b ? x.i : x.k);
+  (void)(b ? x.j : x.k);
+
+  g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+  g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+  g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+  // Hmm...  I don't think these should be accepted.  The conditional
+  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
+  // that are bit-fields, but not lvalues that are conditional
+  // expressions involving bit-fields.
+  h (b ? x.i : x.j);
+  h (b ? x.i : x.k);
+  h (b ? x.j : x.k);
+
+  (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+  (long &)(b ? x.i : x.k); // { dg-error "address of bit-field" }
+  (long &)(b ? x.j : x.k); // { dg-error "address of bit-field" }
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-03-03 12:54 -------
I think this is very much related to PR 19199.  I almost think when that bug is fixed this one will also be 
fixed.

Your current patch will not fix that PR either.

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
  BugsThisDependsOn|                            |19199


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[pinskia at physics dot uc dot edu]

------- Additional Comments From pinskia at physics dot uc dot edu  2005-03-03 12:58 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs


On Mar 3, 2005, at 2:50 AM, Alexandre Oliva wrote:

> I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
> PR c++/20103; it's also passed C++ regression testing.  Ok to install
> if bootstrap and all-languages regression testing passes?

I think this is the wrong approach, we should be doing the same for all
types (well except for bitfields) and not just "addressable" types, see
PR 19199 for a testcase where we get this wrong and there is a proposed
way of fixing this from RTH.

Thanks,
Andrew Pinski



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2005-03-04 01:36 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

Alexandre Oliva wrote:
\
> I went ahead and verified that I didn't break bit-field lvalues in
> conditional expressions (my first attempt did), but I was surprised to
> find out that the calls to h() pass.  I understand why they do (we
> create a temporary and bind to that), but I'm not sure this is correct
> behavior.  Opinions?

> +  // Hmm...  I don't think these should be accepted.  The conditional
> +  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
> +  // that are bit-fields, but not lvalues that are conditional
> +  // expressions involving bit-fields.
> +  h (b ? x.i : x.j);
> +  h (b ? x.i : x.k);
> +  h (b ? x.j : x.k);

That's legal because "h" takes a "const &", which permits the compiler 
to create a temporary.  If it takes a non-const reference, you should 
get an error.

And, I think these kinds of transformations (if necessary) should be 
done in a langhook during gimplification, not at COND_EXPR-creation 
time.  We really want the C++ front-end's data structures to be an 
accurate mirror of the input program for as long as possible.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at redhat dot com]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-04 06:01 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  3, 2005, Mark Mitchell <mark@codesourcery.com> wrote:

> Alexandre Oliva wrote:
> \
>> I went ahead and verified that I didn't break bit-field lvalues in
>> conditional expressions (my first attempt did), but I was surprised to
>> find out that the calls to h() pass.  I understand why they do (we
>> create a temporary and bind to that), but I'm not sure this is correct
>> behavior.  Opinions?

>> +  // Hmm...  I don't think these should be accepted.  The conditional
>> +  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
>> +  // that are bit-fields, but not lvalues that are conditional
>> +  // expressions involving bit-fields.
>> +  h (b ? x.i : x.j);
>> +  h (b ? x.i : x.k);
>> +  h (b ? x.j : x.k);

> That's legal because "h" takes a "const &", which permits the compiler
> to create a temporary.

Yeah, it permits, but only in certain circumstances that AFAICT aren't
met.  This expression AFAICT is an lvalue that isn't a bit-field, so
it has to bind directly, per the first bullet in 8.5.3/5.  Since it
meets the conditions of this first bullet, it doesn't get to use the
`otherwise' portion of that paragraph, that creates a temporary.  Or
am I misreading anything?

> And, I think these kinds of transformations (if necessary) should be
> done in a langhook during gimplification, not at COND_EXPR-creation
> time.  We really want the C++ front-end's data structures to be an
> accurate mirror of the input program for as long as possible.

Err...  But in what sense does my patch change that?  See, what I'm
doing is hoisting the indirect_refs that are inserted by
stabilize_reference out of the cond_expr.  They weren't in the
original code.  There's no dereferencing going on unless the whole
expression undergoes lvalue-to-rvalue decay, so I'd argue that the
transformation I'm proposing actually matches even more accurately the
meaning of the original source code.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[pinskia at gcc dot gnu dot org]

------- Additional Comments From pinskia at gcc dot gnu dot org  2005-03-04 06:30 -------
(In reply to comment #7)
> Yeah, it permits, but only in certain circumstances that AFAICT aren't
> met.  This expression AFAICT is an lvalue that isn't a bit-field, so
> it has to bind directly, per the first bullet in 8.5.3/5.  Since it
> meets the conditions of this first bullet, it doesn't get to use the
> `otherwise' portion of that paragraph, that creates a temporary.  Or
> am I misreading anything?
Yes these expressions are lvalues, see again PR 19199 where we get this wrong.  Even Mark and RTH 
commented on it.  Again it looks like the C++ parser should be creating a tempary to store the address 
and have the indirect reference out of the COND_EXPR, this can only happen for non bitfields.

-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at redhat dot com]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-04 06:34 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  3, 2005, Andrew Pinski <pinskia@physics.uc.edu> wrote:

> On Mar 3, 2005, at 2:50 AM, Alexandre Oliva wrote:

>> I'm bootstrapping this on x86_64-linux-gnu, along with the patch for
>> PR c++/20103; it's also passed C++ regression testing.  Ok to install
>> if bootstrap and all-languages regression testing passes?

> I think this is the wrong approach,

Err...  But AFAICT this is exactly the approach RTH suggested to cope
with the issue, except for the removal of the unnecessary artificial
decl before gimplification.

> we should be doing the same for all types (well except for
> bitfields) and not just "addressable" types,

Agreed.  That's relatively easy to fix.

Improved patch follows.  Ok to install?

Index: gcc/cp/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/19199
	PR c++/20280
	* call.c (build_conditional_expr): Hoist indirect_ref out of
	cond_expr if the result is a non-bitfield lvalue.

Index: gcc/cp/call.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/cp/call.c,v
retrieving revision 1.531
diff -u -p -r1.531 call.c
--- gcc/cp/call.c 24 Feb 2005 21:55:10 -0000 1.531
+++ gcc/cp/call.c 4 Mar 2005 06:32:44 -0000
@@ -3111,6 +3111,9 @@ build_conditional_expr (tree arg1, tree 
   tree result = NULL_TREE;
   tree result_type = NULL_TREE;
   bool lvalue_p = true;
+  bool indirect_p = false;
+  cp_lvalue_kind arg2_lvalue_kind;
+  cp_lvalue_kind arg3_lvalue_kind;
   struct z_candidate *candidates = 0;
   struct z_candidate *cand;
   void *p;
@@ -3288,11 +3291,26 @@ build_conditional_expr (tree arg1, tree 
 
      If the second and third operands are lvalues and have the same
      type, the result is of that type and is an lvalue.  */
-  if (real_lvalue_p (arg2) 
-      && real_lvalue_p (arg3) 
+  if ((arg2_lvalue_kind = real_lvalue_p (arg2))
+      && (arg3_lvalue_kind = real_lvalue_p (arg3))
       && same_type_p (arg2_type, arg3_type))
     {
-      result_type = arg2_type;
+      if ((arg2_lvalue_kind & clk_bitfield) == clk_none
+	  && (arg3_lvalue_kind & clk_bitfield) == clk_none)
+	{
+	  indirect_p = true;
+	  result_type = build_pointer_type (arg2_type);
+	  if (TREE_CODE (arg2) == INDIRECT_REF)
+	    arg2 = TREE_OPERAND (arg2, 0);
+	  else
+	    arg2 = fold_if_not_in_template (build_address (arg2));
+	  if (TREE_CODE (arg3) == INDIRECT_REF)
+	    arg3 = TREE_OPERAND (arg3, 0);
+	  else
+	    arg3 = fold_if_not_in_template (build_address (arg3));
+	}
+      else
+	result_type = arg2_type;
       goto valid_operands;
     }
 
@@ -3458,6 +3476,14 @@ build_conditional_expr (tree arg1, tree 
   /* We can't use result_type below, as fold might have returned a
      throw_expr.  */
 
+  if (indirect_p)
+    {
+      if (TREE_CODE (TREE_TYPE (result)) == POINTER_TYPE)
+	result = build1 (INDIRECT_REF, TREE_TYPE (TREE_TYPE (result)), result);
+      else
+	gcc_assert (TREE_CODE (result) == THROW_EXPR);
+    }
+
   /* Expand both sides into the same slot, hopefully the target of the
      ?: expression.  We used to check for TARGET_EXPRs here, but now we
      sometimes wrap them in NOP_EXPRs so the test would fail.  */
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/19199
	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 4 Mar 2005 06:32:58 -0000
@@ -0,0 +1,80 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+    ~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+    (B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <aoliva@redhat.com>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+  long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+  (b ? x.i : x.j) = 1;
+  (b ? x.j : x.k) = 2;
+  (b ? x.i : x.k) = 3;
+
+  (void)(b ? x.i : x.j);
+  (void)(b ? x.i : x.k);
+  (void)(b ? x.j : x.k);
+
+  g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+  g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+  g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+  // Hmm...  I don't think these should be accepted.  The conditional
+  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
+  // that are bit-fields, but not lvalues that are conditional
+  // expressions involving bit-fields.
+  h (b ? x.i : x.j);
+  h (b ? x.i : x.k);
+  h (b ? x.j : x.k);
+
+  (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+  (long &)(b ? x.i : x.k); // { dg-error "address of bit-field" }
+  (long &)(b ? x.j : x.k); // { dg-error "address of bit-field" }
+}
+
+// What follows is from PR c++/19199, yet another bug on cond-expr
+// lvalues.  We used to return a reference to a temporary in qMin.
+
+enum Foo { A, B };
+
+template<typename T> const T &qMin(const T &a, const T &b) 
+{
+  return a < b ? a : b;
+}
+
+int testref (int,  char **)
+{
+  Foo f = A;
+  Foo g = B;
+  Foo h = qMin(f, g);
+  return 0;
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at redhat dot com]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-04 06:59 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  4, 2005, Alexandre Oliva <aoliva@redhat.com> wrote:

>> we should be doing the same for all types (well except for
>> bitfields) and not just "addressable" types,

> Agreed.  That's relatively easy to fix.

Rats.  Not that easy.  A number of regressions showed up with the
`improved' patch :-(

It has to do with the uses of build_address, that marks variables and
fields as addressable and used, so we end up having to emit them,
instead of optimizing them out as intended.

It seems like we may indeed need something more elaborate at
gimplification time, instead of modifying the up-front representation.

I'll keep digging.



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[mark at codesourcery dot com]

------- Additional Comments From mark at codesourcery dot com  2005-03-04 07:26 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

Alexandre Oliva wrote:

> 
>>>+  // Hmm...  I don't think these should be accepted.  The conditional
>>>+  // expressions are lvalues for sure, and 8.5.3/5 exempts lvalues
>>>+  // that are bit-fields, but not lvalues that are conditional
>>>+  // expressions involving bit-fields.
>>>+  h (b ? x.i : x.j);
>>>+  h (b ? x.i : x.k);
>>>+  h (b ? x.j : x.k);
> 
> 
>>That's legal because "h" takes a "const &", which permits the compiler
>>to create a temporary.
> 
> 
> Yeah, it permits, but only in certain circumstances that AFAICT aren't
> met.  This expression AFAICT is an lvalue that isn't a bit-field, so
> it has to bind directly, per the first bullet in 8.5.3/5.  Since it
> meets the conditions of this first bullet, it doesn't get to use the
> `otherwise' portion of that paragraph, that creates a temporary.  Or
> am I misreading anything?

The situation is a little unclear.

EDG also accepts this code, which is part of what confused me.

Your reading is logical, but it depends on exactly what "lvalue for a 
bit-field" means.  (Note that it does not say "lvalue *is* a bit-field"; 
it says "lvalue *for* a bit-field".)

Consider:

   h((0, x.i));

It would be odd not to allow that, but to allow "h(x.i)".  The 
comma-expression isn't changing what's being passed to "h".  The same 
goes for "h((x.i = 3))".

I think that, if anything, there's a possible defect in the standard 
here, not a defect in the compiler.

>>And, I think these kinds of transformations (if necessary) should be
>>done in a langhook during gimplification, not at COND_EXPR-creation
>>time.  We really want the C++ front-end's data structures to be an
>>accurate mirror of the input program for as long as possible.
>  
> Err...  But in what sense does my patch change that?  See, what I'm
> doing is hoisting the indirect_refs that are inserted by
> stabilize_reference out of the cond_expr.  They weren't in the
> original code.  There's no dereferencing going on unless the whole
> expression undergoes lvalue-to-rvalue decay, so I'd argue that the
> transformation I'm proposing actually matches even more accurately the
> meaning of the original source code.

Actually, looking at this more closely, I think that something is 
forgetting to call rationalize_conditional_expr, which is normally 
called from unary_complex_lvalue.  When the conditional expression is 
used in the lvalue context, something should be calling that.  Normally, 
that happens because something calls build_unary_op (ADDR_EXPR, ...) on 
the COND_EXPR.

It may be that I changed something to call build_addr (instead of 
build_unary_op) in a case where that's not safe.  Can you confirm or 
deny that hypothesis?

Thanks,



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at redhat dot com]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-04 19:23 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Mar  4, 2005, Mark Mitchell <mark@codesourcery.com> wrote:

> Your reading is logical, but it depends on exactly what "lvalue for a
> bit-field" means.  (Note that it does not say "lvalue *is* a
> bit-field"; it says "lvalue *for* a bit-field".)

Good point.  Here's an all-new patch, with the comment updated to
reflect our discussion.

Still testing on x86_64-linux-gnu, ok to install if it succeeds?

Index: gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20280
	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
	temporary variable of pointer type if an lvalues is required.
	(gimplify_modify_expr_rhs): Request an rvalue from it.
	(gimplify_expr): Pass fallback on.

Index: gcc/gimplify.c
===================================================================
RCS file: /cvs/gcc/gcc/gcc/gimplify.c,v
retrieving revision 2.113
diff -u -p -r2.113 gimplify.c
--- gcc/gimplify.c 18 Feb 2005 19:35:37 -0000 2.113
+++ gcc/gimplify.c 4 Mar 2005 19:18:49 -0000
@@ -2123,7 +2123,8 @@ gimple_boolify (tree expr)
      *EXPR_P should be stored.  */
 
 static enum gimplify_status
-gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target)
+gimplify_cond_expr (tree *expr_p, tree *pre_p, tree *post_p, tree target,
+		    fallback_t fallback)
 {
   tree expr = *expr_p;
   tree tmp, tmp2, type;
@@ -2137,18 +2138,40 @@ gimplify_cond_expr (tree *expr_p, tree *
      the arms.  */
   else if (! VOID_TYPE_P (type))
     {
+      tree result;
+
       if (target)
 	{
 	  ret = gimplify_expr (&target, pre_p, post_p,
 			       is_gimple_min_lval, fb_lvalue);
 	  if (ret != GS_ERROR)
 	    ret = GS_OK;
-	  tmp = target;
+	  result = tmp = target;
 	  tmp2 = unshare_expr (target);
 	}
+      else if ((fallback & fb_lvalue) == 0)
+	{
+	  result = tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+	  ret = GS_ALL_DONE;
+	}
       else
 	{
-	  tmp2 = tmp = create_tmp_var (TREE_TYPE (expr), "iftmp");
+	  tree type = build_pointer_type (TREE_TYPE (expr));
+
+	  if (TREE_TYPE (TREE_OPERAND (expr, 1)) != void_type_node)
+	    TREE_OPERAND (expr, 1) =
+	      build_fold_addr_expr (TREE_OPERAND (expr, 1));
+
+	  if (TREE_TYPE (TREE_OPERAND (expr, 2)) != void_type_node)
+	    TREE_OPERAND (expr, 2) =
+	      build_fold_addr_expr (TREE_OPERAND (expr, 2));
+	  
+	  tmp2 = tmp = create_tmp_var (type, "iftmp");
+
+	  expr = build (COND_EXPR, void_type_node, TREE_OPERAND (expr, 0),
+			TREE_OPERAND (expr, 1), TREE_OPERAND (expr, 2));
+
+	  result = build_fold_indirect_ref (tmp);
 	  ret = GS_ALL_DONE;
 	}
 
@@ -2169,7 +2192,7 @@ gimplify_cond_expr (tree *expr_p, tree *
       /* Move the COND_EXPR to the prequeue.  */
       gimplify_and_add (expr, pre_p);
 
-      *expr_p = tmp;
+      *expr_p = result;
       return ret;
     }
 
@@ -2907,7 +2930,8 @@ gimplify_modify_expr_rhs (tree *expr_p, 
 	if (!is_gimple_reg_type (TREE_TYPE (*from_p)))
 	  {
 	    *expr_p = *from_p;
-	    return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p);
+	    return gimplify_cond_expr (expr_p, pre_p, post_p, *to_p,
+				       fb_rvalue);
 	  }
 	else
 	  ret = GS_UNHANDLED;
@@ -3721,7 +3745,8 @@ gimplify_expr (tree *expr_p, tree *pre_p
 	  break;
 
 	case COND_EXPR:
-	  ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE);
+	  ret = gimplify_cond_expr (expr_p, pre_p, post_p, NULL_TREE,
+				    fallback);
 	  break;
 
 	case CALL_EXPR:
Index: gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Index: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
===================================================================
RCS file: gcc/testsuite/g++.dg/tree-ssa/pr20280.C
diff -N gcc/testsuite/g++.dg/tree-ssa/pr20280.C
--- /dev/null	1 Jan 1970 00:00:00 -0000
+++ gcc/testsuite/g++.dg/tree-ssa/pr20280.C 4 Mar 2005 19:19:03 -0000
@@ -0,0 +1,63 @@
+// PR c++/20280
+
+// { dg-do compile }
+
+// Gimplification of the COND_EXPR used to fail because it had an
+// addressable type, and create_tmp_var rejected that.
+
+struct A
+{
+    ~A();
+};
+
+struct B : A {};
+
+A& foo();
+
+void bar(bool b)
+{
+    (B&) (b ? foo() : foo());
+}
+
+// Make sure bit-fields and addressable types don't cause crashes.
+// These were not in the original bug report.
+
+// Added by Alexandre Oliva <aoliva@redhat.com>
+
+// Copyright 2005 Free Software Foundation
+
+struct X
+{
+  long i : 32, j, k : 32;
+};
+
+void g(long&);
+void h(const long&);
+
+void f(X &x, bool b)
+{
+  (b ? x.i : x.j) = 1;
+  (b ? x.j : x.k) = 2;
+  (b ? x.i : x.k) = 3;
+
+  (void)(b ? x.i : x.j);
+  (void)(b ? x.i : x.k);
+  (void)(b ? x.j : x.k);
+
+  g (b ? x.i : x.j); // { dg-error "cannot bind bitfield" }
+  g (b ? x.i : x.k); // { dg-error "cannot bind bitfield" }
+  g (b ? x.j : x.k); // { dg-error "cannot bind bitfield" }
+
+  // It's not entirely clear whether these should be accepted.  The
+  // conditional expressions are lvalues for sure, and 8.5.3/5 exempts
+  // lvalues for bit-fields, but it's not clear that conditional
+  // expressions that are lvalues and that have at least one possible
+  // result that is a bit-field lvalue meets this condition.
+  h (b ? x.i : x.j);
+  h (b ? x.i : x.k);
+  h (b ? x.j : x.k);
+
+  (long &)(b ? x.i : x.j); // { dg-error "address of bit-field" }
+  (long &)(b ? x.i : x.k); // { dg-error "address of bit-field" }
+  (long &)(b ? x.j : x.k); // { dg-error "address of bit-field" }
+}

-- 
Alexandre Oliva             http://www.ic.unicamp.br/~oliva/
Red Hat Compiler Engineer   aoliva@{redhat.com, gcc.gnu.org}
Free Software Evangelist  oliva@{lsd.ic.unicamp.br, gnu.org}


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[jason at redhat dot com]

------- Additional Comments From jason at redhat dot com  2005-03-08 06:47 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Thu, 03 Mar 2005 23:26:04 -0800, Mark Mitchell <mark@codesourcery.com> wrote:

> Your reading is logical, but it depends on exactly what "lvalue for a
> bit-field" means.  (Note that it does not say "lvalue *is* a bit-field"; it
> says "lvalue *for* a bit-field".)

In fact, there's a core issue for exactly that question; Steve's proposed
wording clarifies that these expressions are lvalues for a bit-field.

Jason


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[rth at gcc dot gnu dot org]

------- Additional Comments From rth at gcc dot gnu dot org  2005-03-14 12:59 -------
Subject: Re: [PR c++/20280] hoist indirect_ref out of addressable cond_exprs

On Fri, Mar 04, 2005 at 04:21:53PM -0300, Alexandre Oliva wrote:
> 	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
> 	temporary variable of pointer type if an lvalues is required.
> 	(gimplify_modify_expr_rhs): Request an rvalue from it.
> 	(gimplify_expr): Pass fallback on.

Ok.


r~


-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-03-14 20:02 -------
Subject: Bug 20280

CVSROOT:	/cvs/gcc
Module name:	gcc
Changes by:	aoliva@gcc.gnu.org	2005-03-14 20:02:11

Modified files:
	gcc            : ChangeLog gimplify.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/tree-ssa: pr20280.C 

Log message:
	gcc/ChangeLog:
	PR c++/20280
	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
	temporary variable of pointer type if an lvalues is required.
	(gimplify_modify_expr_rhs): Request an rvalue from it.
	(gimplify_expr): Pass fallback on.
	gcc/testsuite/ChangeLog:
	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&r1=2.7848&r2=2.7849
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/gimplify.c.diff?cvsroot=gcc&r1=2.117&r2=2.118
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&r1=1.5165&r2=1.5166
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr20280.C.diff?cvsroot=gcc&r1=NONE&r2=1.1



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[cvs-commit at gcc dot gnu dot org]

------- Additional Comments From cvs-commit at gcc dot gnu dot org  2005-03-14 20:35 -------
Subject: Bug 20280

CVSROOT:	/cvs/gcc
Module name:	gcc
Branch: 	gcc-4_0-branch
Changes by:	aoliva@gcc.gnu.org	2005-03-14 20:34:53

Modified files:
	gcc            : ChangeLog gimplify.c 
	gcc/testsuite  : ChangeLog 
Added files:
	gcc/testsuite/g++.dg/tree-ssa: pr20280.C 

Log message:
	gcc/ChangeLog:
	PR c++/20280
	* gimplify.c (gimplify_cond_expr): Add fallback argument.  Use a
	temporary variable of pointer type if an lvalues is required.
	(gimplify_modify_expr_rhs): Request an rvalue from it.
	(gimplify_expr): Pass fallback on.
	gcc/testsuite/ChangeLog:
	PR c++/20280
	* g++.dg/tree-ssa/pr20280.C: New.

Patches:
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.7592.2.50&r2=2.7592.2.51
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/gimplify.c.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=2.113.2.1&r2=2.113.2.2
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/ChangeLog.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=1.5084.2.45&r2=1.5084.2.46
http://gcc.gnu.org/cgi-bin/cvsweb.cgi/gcc/gcc/testsuite/g++.dg/tree-ssa/pr20280.C.diff?cvsroot=gcc&only_with_tag=gcc-4_0-branch&r1=NONE&r2=1.1.2.1



-- 


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[aoliva at gcc dot gnu dot org]

------- Additional Comments From aoliva at gcc dot gnu dot org  2005-03-14 23:43 -------
Fixed

-- 
           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |FIXED


http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280

[Bug c++/20280] [4.0/4.1 regression] ICE in create_tmp_var, at gimplify.c:368

[mmitchel at gcc dot gnu dot org]

-- 
Bug 20280 depends on bug 19199, which changed state.

Bug 19199 Summary: [3.3/3.4 Regression] Wrong warning about returning a reference to a temporary
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=19199

           What    |Old Value                   |New Value
----------------------------------------------------------------------------
             Status|NEW                         |ASSIGNED
             Status|ASSIGNED                    |RESOLVED
         Resolution|                            |WONTFIX

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=20280