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

Ulrich Weigand
2008-09-02T21:48:47Z

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 ...