Re: [patch] pr11594

[Chris Moller <cmoller@redhat.com>]

On 06/22/10 14:38, Tom Tromey wrote:
>
> Chris>       case BINOP_COMMA:
> Chris>  -      evaluate_subexp (NULL_TYPE, exp, pos, noside);
> Chris>  -      return evaluate_subexp (NULL_TYPE, exp, pos, noside);
> Chris>  +      arg1 = evaluate_subexp (NULL_TYPE, exp, pos, noside);
> Chris>  +      arg2 = evaluate_subexp (value_type (arg1), exp, pos, noside);
> Chris>  +      if (noside == EVAL_SKIP)
> Chris>  +        goto nosideret;
> Chris>  +      if (current_language->la_language == language_cplus
> Chris>  +	&&  binop_user_defined_p (op, arg1, arg2))
> Chris>  +	{
> Chris>  +	  struct value *rc;
> Chris>  +	
> Chris>  +	  rc = value_x_binop (arg1, arg2, op, OP_NULL, noside);
> Chris>  +	  if (rc != NULL)
> Chris>  +	    return rc;
> Chris>  +	  else
> Chris>  +	    return arg2;
>
> I think I understand why the current_language check and the result check
> of value_x_binop are needed.
>
> However, I think it would be better to do all the work in value_x_binop
> and also remove the current_language check.  This is more similar to
> what other code does and it consolidates the (broken) current_language
> checks in the value code.
>
> I'm guessing this means a change to value_user_defined_op.
>    

I went down that route--that's what's taken so long about getting this 
patch out.  Under some circumstances, you wind up special-case checking 
for BINOP_COMMA and strcmp("operator,"   ) all over the place and 
eventually wind up in a place where there's not enough contextual 
information left to determine if you're dealing with an overloaded comma 
and call value_addr() that tries to take the target address of a 
register value.

The value_x_binop() result is checked because binop_user_defined_p() 
only asserts that it's possible for the operands to represent an 
overload, not that there is in fact an overload in this instance.  
That's apparently determined in value_x_binop() and if the operator 
isn't overloaded, I'm defaulting back to non-overloaded behaviour.  (It 
seems to me that the language  check ought to be in 
binop_user_defined_p() or binop_types_user_defined_p()--is there ever a 
circumstance other than language_cplus  where you can have overloaded 
operators?)

If the language check really is unacceptable, I'll revisit this, but the 
patch gets significantly more complicated.

I'm going to back-burner this, though--I've gone back to the matrix 
pretty-print stuff for a while.

-cm