From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 115265 invoked by alias); 26 Aug 2019 02:20:23 -0000 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 Received: (qmail 115019 invoked by uid 89); 26 Aug 2019 02:20:23 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-9.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_1,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.1 spammy=factor, our X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Mon, 26 Aug 2019 02:20:20 +0000 Received: from [10.0.0.193] (unknown [192.222.164.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 05B381E65F; Sun, 25 Aug 2019 22:20:18 -0400 (EDT) Subject: Re: [PATCH v2] Factor out the common code in lookup_{static,global}_symbol To: Christian Biesinger , gdb-patches@sourceware.org References: <20190805153326.95512-1-cbiesinger@google.com> From: Simon Marchi Message-ID: <45f47d3b-c9f5-5419-517f-31a4a4b5d046@simark.ca> Date: Mon, 26 Aug 2019 02:20:00 -0000 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.8.0 MIME-Version: 1.0 In-Reply-To: <20190805153326.95512-1-cbiesinger@google.com> Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: 7bit X-SW-Source: 2019-08/txt/msg00582.txt.bz2 On 2019-08-05 11:33 a.m., Christian Biesinger via gdb-patches wrote: > @@ -2614,6 +2576,9 @@ struct global_sym_lookup_data > /* The domain to use for our search. */ > domain_enum domain; > > + /* The block index in which to search */ Period and two spaces: /* The block index in which to search. */ > -struct block_symbol > -lookup_global_symbol (const char *name, > - const struct block *block, > - const domain_enum domain) > +static struct block_symbol > +lookup_global_symbol_internal (const char *name, > + enum block_enum block_index, > + const struct block *block, > + const domain_enum domain) I'd rename that to lookup_symbol_internal, since it's used both in lookup_global_symbol and lookup_static_symbol. If you think it's too general (the function doesn't look up local symbols), Maybe "lookup_global_or_static_symbol"? And global_sym_lookup_data should be renamed accordingly. And since BLOCK is only used for looking up an objfile in the global case, I would suggest making this function accept the objfile instead of a block. Otherwise, it could be confused as "lookup symbol in this block". Also, if some code already has the objfile handy, they can pass it down, in which case we avoid the cost of lookup_objfile_from_block. > +/* See symtab.h. */ > + > +struct block_symbol > +lookup_static_symbol (const char *name, const domain_enum domain) > +{ > + /* TODO: Should static symbol lookup start with a block as well, so we can > + prefer a static symbol in the current CU? */ > + return lookup_global_symbol_internal (name, STATIC_BLOCK, nullptr, domain); I'd say that we can add another argument when the need comes, it's not a stable API. Simon