Re: [patch] fix pre-/post- in-/decrement

["Ulrich Weigand" <uweigand@de.ibm.com>]

Ken Werner wrote:

> -	  return value_assign (arg1, arg2);
> +	  /* Prevent to return a lvalue.  */
> +	  arg3 = value_assign (arg1, arg2);
> +	  VALUE_LVAL (arg3) = not_lval;
> +	  return arg3;

We want to get away from changing core properties like lval
in values after the fact ...   In any case, hard-coding the
lval to non_lval without any further change can cause problems,
e.g. if the value is lazy.

I think there is a more general issue underlying this particular
change.  You're right that the result of a preincrement should
not be an lvalue.  But the same is true for results of assignment
operators in general.

Note that value_assign is used only to implement such operators
(simple assignment, compound assignment, pre/postfix operators).
Since *all* of them return non-lvalues, it might make sense to
simply change value_assign to return a non-lvalue in the first
place ...


> +	  type = value_type (arg1);
> +	  arg3 = allocate_value (type);
> +
> +	  /* Copy the value to prevent to return a lvalue.  */
> +	  memcpy (value_contents_raw (arg3), value_contents (arg1),
> +		  TYPE_LENGTH (type));

I'd prefer to encapsulate this in a function, e.g. value_non_lval (...)
or a similar name, which returns a version of the value that is non_lval.
This could then address a couple of additional issues:
- if the value is already non_lval, no need to create an extra copy
- it might be better to copy the full contents / enclosing type
  for C++ objects

Bye,
Ulrich

-- 
  Dr. Ulrich Weigand
  GNU Toolchain for Linux on System z and Cell BE
  Ulrich.Weigand@de.ibm.com