Re: [patch] smart pointer support

[sami wagiaalla <swagiaal at redhat dot com>]

On 07/23/2010 07:37 PM, Tom Tromey wrote:
> > > > > > "Sami" == sami wagiaalla<swagiaal@redhat.com>  writes:
>
> Sami>  The '->' operator can be overloaded in C++ and when overloaded it
> Sami>  forwards the function call or member reference to the return
> Sami>  type.
>
> Thanks for working on this.

Np. This was fun :)

> Sami>  +	      /* For C++ check to see if the operator '->' has been overloaded.
> Sami>  +	         If the operator has been overloaded replace arg2 with the value
> Sami>  +	         returned by the custom operator and continue evaluation.  */
> Sami>  +	      if (exp->language_defn->la_language == language_cplus)
> Sami>  +		{
> Sami>  +		  struct type *arg_type = value_type (arg2);
> Sami>  +		  if (arg_type&&  TYPE_CODE (arg_type) == TYPE_CODE_STRUCT)
>
> I think if you are going to use value_x_unop, you ought to use
> unop_user_defined_p as well.  Maybe that means changes to
> unop_user_defined_p.

Done. I avoided it in the beginning because I thought I wouldn't need to 
check for TYPE_CODE (arg_type) == TYPE_CODE_STRUCT but ended up needing 
it and never went back to unop_user_defined_p.

> Also, I think you will need a while loop here.  operator->  might itself
> return an object that has an operator->.  There should be a test case
> for this as well.

This turned out to be a little hairy when operator->() returns a copy 
instead of a reference. Some flavours of this are easy to fix by setting 
the VALUE_LVAL of the value returned by call_function_by_hand to 
lval_memory, and other cases are harder. I have spent a day looking at 
this but couldn't come up with anything. So I added a test and marked it 
as kfail and update the bug. I also, added tests for the cases the work.

> I was wondering if the STRUCTOP_PTR / STRUCTOP_STRUCT conflation in the
> method case could cause problems.  Maybe a new test would be good, just
> to make that clear.  The "." case should not cause the operator to be
> used -- or at least, "." on a struct, gdb supports "." on a pointer as
> an extension, I'm not sure what we should do there.

I added test for the '.' case both the extension and the plain '.' there 
are also tests in the patch for none overloaded -> operator

> Otherwise, looking good.

I will wait until the template stuff is in before committing this. The 
template test cases in this patch might fail otherwise.

Thanks for the review.

Sami

Attachment: smart_pointer.patch
Description: Text document