Re: Patch for PR 9399

[Chris Moller <cmoller@redhat.com>]

On 12/10/09 14:04, Tom Tromey wrote:
>>>>>> "Daniel" == Daniel Jacobowitz<drow@false.org>  writes:
>>>>>>              
>
> Daniel>  Tom's approved this.  That's fine.
>
> I'm open to unapproving it if you think it is incorrect.
> I don't want to paper over a bug.
>
>    
>>> value_cast_structs only does nothing if both TYPE_NAME()s are null.
>>> I was wondering if, back when the code was originally written, if
>>> there never was a case when both TYPE_NAME()s were non-null--it's the
>>> only way, other than simple oversight, I can account for that case
>>> not being covered.
>>>        
>
> Daniel>  No, it looks exactly like I intended.  If we have two classes, we
> Daniel>  check if A is a base class of B, then if B is a base class of A.
> Daniel>  If A == B, neither of those checks will be true, and we'll return
> Daniel>  without doing anything.
>
> Daniel>  So the problem is that one of those base class checks is calling
> Daniel>  error().  That's not supposed to happen.
>
> I think from value_cast_structs:
>
>    /* Downcasting: look in the type of the target to see if it contains the
>       type of the source as a superclass.  If so, we'll need to
>       offset the pointer rather than just change its type.
>       FIXME: This fails silently with virtual inheritance.  */
>
> ... but this doesn't fail silently, it tries to dereference NULL, due to
> the use of value_zero.  IIRC.
>    

Yeah, that's what it does, but I've forgotten exactly where in the code 
I saw it doing that.

> The reason I approved this is that it seems strange to me to try to do
> any cast when we can tell beforehand that it is not needed.  It is doing
> a lot of work for no reason.
>
> Also, I tried a little to write a different test that would still hit
> this failure, and I couldn't.  I don't know if Chris tried this or not.
>    

Nope.

> Tom
>