From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 25440 invoked by alias); 13 Oct 2009 19:47:18 -0000 Received: (qmail 25431 invoked by uid 22791); 13 Oct 2009 19:47:18 -0000 X-SWARE-Spam-Status: No, hits=-2.5 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS X-Spam-Check-By: sourceware.org Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.43rc1) with ESMTP; Tue, 13 Oct 2009 19:47:14 +0000 Received: from int-mx03.intmail.prod.int.phx2.redhat.com (int-mx03.intmail.prod.int.phx2.redhat.com [10.5.11.16]) by mx1.redhat.com (8.13.8/8.13.8) with ESMTP id n9DJlDkh002621 for ; Tue, 13 Oct 2009 15:47:13 -0400 Received: from ns3.rdu.redhat.com (ns3.rdu.redhat.com [10.11.255.199]) by int-mx03.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id n9DJlCqe001952; Tue, 13 Oct 2009 15:47:12 -0400 Received: from opsy.redhat.com (ovpn01.gateway.prod.ext.phx2.redhat.com [10.5.9.1]) by ns3.rdu.redhat.com (8.13.8/8.13.8) with ESMTP id n9DJlBp6006507; Tue, 13 Oct 2009 15:47:11 -0400 Received: by opsy.redhat.com (Postfix, from userid 500) id CBAA6378188; Tue, 13 Oct 2009 13:47:10 -0600 (MDT) From: Tom Tromey To: Sami Wagiaalla Cc: GDB Patches Subject: Re: [patch 1/2] Perform a namespace lookup at every block level References: <4A57512A.7090208@redhat.com> <20090710194949.GA2064@caradoc.them.org> <4A5B68A4.30006@redhat.com> <4A68B91D.2080206@redhat.com> <4A8B0FBA.4090501@redhat.com> Reply-To: tromey@redhat.com Date: Tue, 13 Oct 2009 19:47:00 -0000 In-Reply-To: <4A8B0FBA.4090501@redhat.com> (Sami Wagiaalla's message of "Tue, 18 Aug 2009 16:31:54 -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 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org X-SW-Source: 2009-10/txt/msg00284.txt.bz2 >>>>> "Sami" == Sami Wagiaalla writes: Sami> Same as above, but I fixed a couple of issues with the previous patch. Sami> The first issue is that even while we iterate over blocks the original Sami> scope should be used. Secondly, the call to lookup_symbol_name space Sami> in valops.c must also perform a per block search. The final issue is Sami> recursive search which is discussed in part 2 of this patch. Sorry about the delay on this. For best results, ping a pending patch every week or two -- the lack of a response can mean either that everybody is busy or that everybody somehow missed the posting entirely, and there's no way to tell the difference. Sami> @@ -265,8 +277,41 @@ cp_lookup_symbol_nonlocal (const char *name, [...] Sami> + if ( sym != NULL) Extra space after the "(". Sami> + return cp_lookup_symbol_namespace(scope, name, linkage_name, block, domain); Missing a space before "(". There are several of these. Sami> +static struct symbol * Sami> +cp_lookup_symbol_imports (const char *scope, Sami> + const char *name, Sami> + const char *linkage_name, Sami> + const struct block *block, Sami> + const domain_enum domain) [...] Sami> + if ( sym != NULL) The extra space again :) Sami> @@ -1304,13 +1304,14 @@ lookup_symbol_aux (const char *name, const char *linkage_name, [...] Sami> + /* C++ language specific lookup requires the lowest block level. */ Sami> + if (language != language_cplus) Sami> + block = function_block; I like to avoid special cases for a specific language in the generic code. These make gdb more brittle over time; also they make it harder to understand. One alternative approach would be to pass `block' as an argument to the la_lookup_symbol_nonlocal method. This would mean adding an ignored parameter to most of the implementations of this method, but that is no big deal, we've done that sort of thing before. Sami> diff --git a/gdb/testsuite/gdb.cp/namespace-using.cc b/gdb/testsuite/gdb.cp/namespace-using.cc I think this needs more tests. At the very least it should have a test case for the bug Daniel pointed out upthread, but I think it would be good to have tests for other confounding cases. Tom