On Fri, Apr 22, 2016 at 2:12 PM, Marek Polacek <pola...@redhat.com> wrote: > On Fri, Apr 22, 2016 at 09:43:31AM -0400, Jason Merrill wrote: >> On 04/22/2016 07:50 AM, Marek Polacek wrote: >> >This PR shows that we generate wrong code with the x ?: y extension in case >> >the >> >first operand contains either predecrement or preincrement. The problem is >> >that we don't emit SAVE_EXPR, thus the operand is evaluated twice, which it >> >should not be. >> > >> >While ++i or --i can be lvalues in C++, i++ or i-- can not. The code in >> >build_conditional_expr_1 has: >> > 4635 /* Make sure that lvalues remain lvalues. See >> > g++.oliva/ext1.C. */ >> > 4636 if (real_lvalue_p (arg1)) >> > 4637 arg2 = arg1 = stabilize_reference (arg1); >> > 4638 else >> > 4639 arg2 = arg1 = save_expr (arg1); >> >so for ++i/--i we call stabilize_reference, but that doesn't know anything >> >about PREINCREMENT_EXPR or PREDECREMENT_EXPR and just returns the same >> >expression, so SAVE_EXPR isn't created. >> > >> >I think one fix would be to teach stabilize_reference what to do with those, >> >similarly to how we handle COMPOUND_EXPR there. >> >> Your change will turn the expression into an rvalue, so that isn't enough. > > Oops. > >> We need to turn the expression into some sort of _REF before passing it to >> stabilize_reference, perhaps by factoring out the cast-to-reference code >> from force_paren_expr. This should probably be part of a >> cp_stabilize_reference function that replaces all uses of >> stabilize_reference in the front end. > > Thanks, this magic seems to work. So something like the following? I didn't > put the cast-to-reference code into its separate function because it didn't > seem convenient, but I can work on that, too, if you prefer.
> +cp_stabilize_reference (tree ref) > +{ > + if (TREE_CODE (ref) == PREINCREMENT_EXPR > + || TREE_CODE (ref) == PREDECREMENT_EXPR) I think we want to do this for anything stabilize_reference doesn't handle specifically, not just pre..crement. Jason