Re: [rfc][08/37] Eliminate builtin_type_ macros: Make pointer arithmetic explicit

["Ulrich Weigand" <uweigand at de dot ibm dot com>]

Daniel Jacobowitz wrote:

> > This moves the type to be used for pointer difference operations up
> > eval.c (this should at some point be replaces by a per-gdbarch 
> > "ptrdiff_t" type).  It also makes the subsequent patches to remove
> > current_gdbarch references from value_binop simpler.
> 
> I've got to say I don't like the direction of this change :-(
> 
> Smarts in the expression parser are not available to reuse by other
> code that constructs values - which is something I think we should be
> doing more often.  Why should the caller have to know that the two
> values are pointers?

Lets put the question differently:  Why should the generic "add" routine
of a debugger supporting many languages have hard-coded semantics that
are specific to C (and in fact, the C ABI on a specific platform)?

I was trying to make the "value_*" routines be as much as possible
language- and architecture-independent, and push language- and
architecture-specific semantics up to higher layers.  (In this case,
the expression evaluator.  In fact, I might like it even better if 
expressions themselves were also language-agnostic, and all the
language-specific semantics were encoded explicitly into different
operand codes by the parsers ...)

As to your question: when replacing uses of value_add, every caller
*knew* whether the arguments were pointers or scalars (exept for the
generic expression evaluator, of course).  GDB-internal uses do not
really assume the C-specific overloading of the "+" operator ...

> > @@ -1531,8 +1556,19 @@ evaluate_subexp_standard (struct type *e
> >  	goto nosideret;
> >        if (binop_user_defined_p (op, arg1, arg2))
> >  	return value_x_binop (arg1, arg2, op, OP_NULL, noside);
> > +      else if (ptrmath_type_p (value_type (arg1))
> > +	       && ptrmath_type_p (value_type (arg2)))
> > +	{
> > +	  /* FIXME -- should be ptrdiff_t */
> > +	  type = builtin_type (exp->gdbarch)->builtin_long;
> > +	  return value_from_longest (type, value_ptrdiff (arg1, arg2));
> > +	}
> > +      else if (ptrmath_type_p (value_type (arg1)))
> > +	return value_ptrsub (arg1, arg2);
> > +      else if (ptrmath_type_p (value_type (arg2)))
> > +	return value_ptrsub (arg2, arg1);
> >        else
> > -	return value_sub (arg1, arg2);
> > +	return value_binop (arg1, arg2, BINOP_SUB);
> >  
> >      case BINOP_EXP:
> >      case BINOP_MUL:
> 
> There's something wrong in the last else if; arg1 - arg2 != arg2 - arg1.

Oops.  That last case "integer - pointer" is not allowed in C anyway;
this should throw an error (the old code did as well; this is a bug
introduced by my change).  I'll fix that ...

Bye,
Ulrich

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