On 10/12/2010 04:40 PM, Tom Tromey wrote:
>>>>>> "Sami" == sami wagiaalla<swagiaal@redhat.com>  writes:
>
> Sami>  I understood that one could convert 'Class ***' to 'BaseClass ***',
> Sami>  but it turns out that is in correct. I should not have assumed that
> Sami>  without testing, and there is nothing in the spec that should have
> Sami>  made me think so. Since that is the case, the calling function can
> Sami>  just deference the pointers.
>
> Is there a test for this?  There should be.  (I didn't check.)
>

I added one. The test attempts to make the conversion and fails as expected.

> Tom>  I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
> Tom>  value and instead introduced BASE_PTR_CONVERSION_BADNESS.
>
> Tom>  But then, I also don't understand the existing code that returns
> Tom>  POINTER_CONVERSION_BADNESS...
>
> Sami>  base pointer conversion (BASE_PTR_CONVERSION_BADNESS) is meant to be a
> Sami>  slightly better option than generic (POINTER_CONVERSION_BADNESS)
>
> I still don't understand.
>
> I guess POINTER_CONVERSION_BADNESS is used for really bogus operations,
> like converting an int to a pointer.  It seems to be a gdb extension.
>
> I'm not sure this is worth supporting.
>

A pointer can be legally converted to a base pointer a void pointer or a 
boolean. These are ranked in that order. All other conversions are 
indeed bogus. To rank these we use:

BASE_PTR_CONVERSION_BADNESS 1
VOID_PTR_CONVERSION_BADNESS 2
POINTER_CONVERSION_BADNESS  2

Note: Converting a pointer to a matching array is considered an exact 
match (rank 0).

Having typed this out, I think POINTER_CONVERSION_BADNESS should be 
renamed to BOOL_PTR_CONVERSION_BADNESS and given a rank of 3. And all 
other conversions outlawed. I'll do this in and test it another patch. 
This patch really only deals with the first clauses of the nested switch 
statements.

I totally agree on not supporting the other conversions as extensions.

> Tom>  Also, why specifically is_public_ancestor and not is_ancestor?
>
> Sami>  You can convert a pointer to B to a pointer to A only if A is an
> Sami>  accessible ancestor of B.
>
> GDB generally ignores access protection.  It seems like it ought to here
> as well.
>

Hmm, I am worried that this will make overload resolution more 
complicated, and lead to incorrect resolution. I have no objection to 
calling the inaccessible base function if the user casts the the type:
B b;
((A)b).foo();
or
foo((A)b))
but in overload resolution I think we should stick to the standard at 
least until we are more confident in our implementation.

> Sami>  +  /* If a and b are both pointers types or both reference types then
> Sami>  +     they are equal of the same type iff the objects they refer to are
> Sami>  +     of the same type.  */
> Sami>  +  if (TYPE_CODE (a) == TYPE_CODE_PTR
> Sami>  +      || TYPE_CODE (a) == TYPE_CODE_REF)
> Sami>  +    return types_equal (TYPE_TARGET_TYPE (a),
> Sami>  +                        TYPE_TARGET_TYPE (b));
>
> This recursive call seems a little odd.
> The existence of a check for the "Class ***" case would help prove that
> it is ok, though.
>

I added the 'Class ***' check as requested, and I should point out that 
there is also foo2_2, foo2_3, and foo2_4 to test this spot of the code.