From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 19748 invoked by alias); 27 Aug 2009 21:49:46 -0000 Mailing-List: contact archer-help@sourceware.org; run by ezmlm Sender: Precedence: bulk List-Post: List-Help: List-Subscribe: List-Id: Received: (qmail 19739 invoked by uid 22791); 27 Aug 2009 21:49:46 -0000 X-SWARE-Spam-Status: No, hits=-1.8 required=5.0 tests=AWL,BAYES_00,J_CHICKENPOX_44,J_CHICKENPOX_54,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org From: Tom Tromey To: Sami Wagiaalla Cc: Project Archer Subject: Re: [RFC] Koenig lookup patch 2 References: <49BABABE.9080606@redhat.com> <49F87751.8050405@redhat.com> <4A969900.6040100@redhat.com> Reply-To: Tom Tromey Date: Thu, 27 Aug 2009 21:49:00 -0000 In-Reply-To: <4A969900.6040100@redhat.com> (Sami Wagiaalla's message of "Thu, 27 Aug 2009 10:32:32 -0400") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-SW-Source: 2009-q3/txt/msg00158.txt.bz2 >>>>> "Sami" == Sami Wagiaalla 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