Re: [patch 2/2] Fix overload resolution of int* vs void*

[Tom Tromey <tromey@redhat.com>]

>>>>> "Sami" == sami wagiaalla <swagiaal@redhat.com> writes:

Sami> @@ -1886,6 +1886,14 @@ do_is_ancestor (struct type *base, struct type *dclass, int public)
Sami>    if (class_types_same_p (base, dclass))
Sami>      return 1;
 
Sami> +  if ((TYPE_CODE (base) == TYPE_CODE_PTR
Sami> +       && TYPE_CODE (dclass) == TYPE_CODE_PTR)
Sami> +       || (TYPE_CODE (base) == TYPE_CODE_REF
Sami> +	   && TYPE_CODE (dclass) == TYPE_CODE_REF))
Sami> +    return do_is_ancestor (TYPE_TARGET_TYPE (base),
Sami> +                           TYPE_TARGET_TYPE (dclass),
Sami> +                           public);

This seems strange to me.  It ought to only be applicable at the
outermost call.  But in that case, I think a wrapper function would be
more appropriate, so that do_is_ancestor has a single purpose.

Also, is this the right thing for the caller when there are multiple
pointers (like "Class ***")?  Or do you intend to only strip a single
pointer?

Sami> +  /* Resolve typedefs */
Sami> +  if (TYPE_CODE (a) == TYPE_CODE_TYPEDEF)
Sami> +    a = check_typedef (a);
Sami> +  if (TYPE_CODE (b) == TYPE_CODE_TYPEDEF)
Sami> +    b = check_typedef (b);

It is ok to call check_typedef unconditionally, but I see this is just
copied from rank_one_type.

Sami> +	  /* (b) pointer to ancestor-pointer conversion.  */
Sami> +	  if (is_public_ancestor (parm, arg))
Sami> +	    return BASE_PTR_CONVERSION_BADNESS;

I am curious why you didn't just give POINTER_CONVERSION_BADNESS a new
value and instead introduced BASE_PTR_CONVERSION_BADNESS.

But then, I also don't understand the existing code that returns
POINTER_CONVERSION_BADNESS...

Also, why specifically is_public_ancestor and not is_ancestor?

I suppose this is an incremental patch -- still not truly correct with
regards to the distance-to-base problem, but an improvement over what
gdb currently does.  This is fine, even great, but it is handy for
reviewers if you note that sort of thing in the patch explanation.

Sami>  	case TYPE_CODE_ARRAY:
Sami> -	  return rank_one_type (TYPE_TARGET_TYPE (parm), 
Sami> -				TYPE_TARGET_TYPE (arg));
Sami> +	  if (types_equal (TYPE_TARGET_TYPE (parm),
Sami> +	                   TYPE_TARGET_TYPE (arg)))
Sami> +	    return 0;
Sami> +	  return INCOMPATIBLE_TYPE_BADNESS;

Do the new tests cover this case?  I could not immediately tell.

Tom