Re: [RFC] Extend existing support for evaluating expressions using overloaded operators

[Siva Chandra <sivachandra at google dot com>]

Siva> ? ? ? ? * valarith.c (value_binop): Extend to handle overloaded
Siva> ? ? ? ? binary operations on compound types.

Tom> I don't understand why value_binop must be modified.
Tom> I think in the current design it is up to the caller to check this.
Tom> (I don't necessarily think this is a good design -- but changing it
Tom> would require more changes elsewhere.)

If we want to extend the operator overloading support into Python,
then we will have to have code to decide whether to call value_binop
or value_x_binop in the py-value.c. Another client of these two
functions is evaluate_subexp_standard, which also has code to decide
whether to call value_binop or value_x_binop. Though I did not mention
in my post, my intention is avoid such repetitive code and get all
clients to depend only on value_binop and treat value_x_binop as
'internal'. I would like to follow this patch with another patch to do
this change. Note that, only evaluate_subexp_standard calls
value_x_binop currently. Hence, replacing these calls with value_binop
should not be much work (though I have not yet come up with a way to
deal with the 'noside' argument of value_x_binop).

Siva> +int
Siva> +is_compound_type (struct type *type)
Siva> +{
Siva> + ?enum type_code type_code;
Siva> +
Siva> + ?CHECK_TYPEDEF (type);
Siva> +
Siva> + ?type_code = TYPE_CODE (type);
Siva> + ?if (type_code == TYPE_CODE_STRUCT || type_code == TYPE_CODE_UNION)

Tom> It seems to me that there is other code which assumes that only
Tom> TYPE_CODE_STRUCT can be overloaded. ?At least binop_types_user_defined_p,
Tom> but maybe others. ?Those spots should be updated.

Actually, AFAICT binop_types_user_defined_p has a copy-paste typo due
to which it is currently wrong. Even otherwise, with the change I have
in mind this function can go away I think.

Tom> It would be nice if the test suite tested this case as well.

I agree, I will add more tests after we finalize this patch.

Siva> + ? ? ?TRY_CATCH (except, RETURN_MASK_ERROR)
Siva> + ? ? ? ?{
Siva> + ? ? ? ? ?/* Retrieve the list of methods with the name NAME. ?*/
Siva> + ? ? ? ? ?fns_ptr = value_find_oload_method_list (&temp, name,
Siva> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? 0, &num_fns,
Siva> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&basetype, &boffset);
Siva> + ? ? ? ?}
Siva> + ? ? ?if (except.reason < 0)
Siva> + ? ? ? ?fns_ptr = NULL;

Tom> I'll have to go read this in more depth; but I wonder why it is ok to
Tom> ignore exceptions here.

An exception is thrown if we are trying to lookup methods of a
non-struct and non-union type. One can ask that, in such cases, why
call value_find_oload_method_list at all. My argument is, if
value_find_oload_method_list is doing the check, why should we repeat
before calling it.

Thanks,
Siva Chandra