Re: [RFC] Koenig lookup patch 2

[Tom Tromey <tromey@redhat.com>]

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

Tom> there were
Tom> some formatting issues and whatnot -- these have to be fixed but are
Tom> not very important overall.

This comment still applies.  Please fix the formatting problems.
I can list them if you want.

Tom> With the current code is there a need to have the values here?
Tom> Or could this be reverted to the trunk's approach?

Sami> This was my attempt of making the code a little more compact.

Ah, I see.. thanks.

Tom> Does this really do the right thing in the case where the call has
Tom> multiple arguments, each of which has a type from a different
Tom> namespace?  I don't understand how those would get added to the
Tom> overload set.

Sami> The overload resolution is performed by the remainder of the function
Sami> (find_overloaded_match) this is just to give a starting point. However,
Sami> as you suspected, find_overload_match assumes that all the overload
Sami> choices are within the same namespace. This is corrected in this patch.

I am still not convinced this is correct.

First, I think Koenig lookup looks in the namespaces of all the
"associated" classes of each arguments.  So, if you have an argument of
type one::A which inherits from two::B, I think Koenig lookup should
search both one:: and two::.  I don't see that in this patch.  (I didn't
look this up in the standard, so if I'm wrong about it, feel free to say
so :-)

Sami> +			  /* This could potentially be a an argument dependet

Typo, "dependent".

Sami> +			     lookup function (koenig).  */

Should capitalize Koenig.

Sami> +    if (sym == NULL && !lookup_minimal_symbol (tmp, NULL, NULL) && !is_a_field_of_this)

I think checking is_a_field_of_this is unneeded here.

Sami> +void
Sami> +find_adl_match (struct type **arg_types, int nargs,char *name,
Sami> +               struct symbol **symp)
Sami> +{
[...]
Sami> +  for (i = 1; i <= nargs; i++){
[...]
Sami> +      if ( fsym &&
Sami> +           find_overload_match(arg_types, nargs, name, 0,
Sami> +                              0, NULL, fsym,NULL, symp,NULL) == 0)

I am not sure this is correct either.

My understanding is that in Koenig lookup, all the functions found this
way are put into an overload set which is then subject to overload
resolution rules.  ("Top-level" functions found by ordinary lookup are
also put into this set.)

In this code, I think you are just searching each namespace separately
for the best overload.  This seems like it could yield the wrong answer
in some cases (or at least, yield an answer when in fact the request is
ambiguous).

Tom