From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 58886 invoked by alias); 31 Aug 2015 13:55:15 -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 58850 invoked by uid 89); 31 Aug 2015 13:55:15 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.2 required=5.0 tests=AWL,BAYES_00,SPF_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 X-HELO: e06smtp11.uk.ibm.com Received: from e06smtp11.uk.ibm.com (HELO e06smtp11.uk.ibm.com) (195.75.94.107) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (CAMELLIA256-SHA encrypted) ESMTPS; Mon, 31 Aug 2015 13:55:13 +0000 Received: from /spool/local by e06smtp11.uk.ibm.com with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted for from ; Mon, 31 Aug 2015 14:55:10 +0100 Received: from d06dlp03.portsmouth.uk.ibm.com (9.149.20.15) by e06smtp11.uk.ibm.com (192.168.101.141) with IBM ESMTP SMTP Gateway: Authorized Use Only! Violators will be prosecuted; Mon, 31 Aug 2015 14:55:08 +0100 X-MailFrom: uweigand@de.ibm.com X-RcptTo: gdb-patches@sourceware.org Received: from b06cxnps4074.portsmouth.uk.ibm.com (d06relay11.portsmouth.uk.ibm.com [9.149.109.196]) by d06dlp03.portsmouth.uk.ibm.com (Postfix) with ESMTP id 75C0D1B0805F for ; Mon, 31 Aug 2015 14:56:42 +0100 (BST) Received: from d06av01.portsmouth.uk.ibm.com (d06av01.portsmouth.uk.ibm.com [9.149.37.212]) by b06cxnps4074.portsmouth.uk.ibm.com (8.14.9/8.14.9/NCO v10.0) with ESMTP id t7VDt5Te13369566 for ; Mon, 31 Aug 2015 13:55:08 GMT Received: from d06av01.portsmouth.uk.ibm.com (localhost [127.0.0.1]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVout) with ESMTP id t7VDstci016021 for ; Mon, 31 Aug 2015 07:54:56 -0600 Received: from oc7340732750.ibm.com (dyn-9-152-213-24.boeblingen.de.ibm.com [9.152.213.24]) by d06av01.portsmouth.uk.ibm.com (8.14.4/8.14.4/NCO v10.0 AVin) with ESMTP id t7VDsteI015697; Mon, 31 Aug 2015 07:54:55 -0600 Received: by oc7340732750.ibm.com (Postfix, from userid 500) id B63D68126; Mon, 31 Aug 2015 15:54:46 +0200 (CEST) Subject: Re: Cell multi-arch type resolution broken (Re: [PATCH 5/6] [PR 17684] add support for primitive types as symbols) To: xdje42@gmail.com (Doug Evans) Date: Mon, 31 Aug 2015 13:55:00 -0000 From: "Ulrich Weigand" Cc: gdb-patches@sourceware.org In-Reply-To: from "Doug Evans" at Aug 30, 2015 09:15:55 PM MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit Message-Id: <20150831135446.B63D68126@oc7340732750.ibm.com> X-TM-AS-MML: disable X-Content-Scanned: Fidelis XPS MAILER x-cbid: 15083113-0041-0000-0000-000005A5EF19 X-SW-Source: 2015-08/txt/msg00867.txt.bz2 Doug Evans wrote: > > I'd be ok with adding a gdbarch parameter to lookup_symbol, > > and require at least one of block or gdbarch to be non-NULL. > > > > The symbol lookup code is a lot simpler when block == NULL, > > and handling all the different cases in one set of functions > > makes things more complex than they could otherwise be. > > One might then split things up into two paths underneath > > (one for block, one for arch). > > I went through and played with adding gdbarch to the > symbol lookup routines. I pushed into some places that I didn't > I need to, just to see the effect. > I'll clean this up and resubmit in a bit. Thanks! > This patch also allows block == NULL and gdbarch == NULL > in the call to lookup_symbol. Sometimes the caller doesn't have either > one and doesn't need either one. I'd prefer something else though: > It'd be cleaner to remove the choice and require the caller to use > routines that are more explicit. > Otherwise I suspect a subtle bug or two will creep in. > [IOW, if the lookup may be for an arch-specific symbol (e.g., primitive type) > then require the caller to use a routine that requires a non-NULL gdbarch. > Internal to symtab.c we can do whatever we want though.] Absolutely. I agree that it would be preferable to define the interface so that if you want to enable looking up primitive types, you need to pass in an gdbarch explicitly. Conversely, many places know for sure they do not need primitive types (e.g. because they are looking up a variable or function name, or they are looking up some non-primitive type like a struct type), and those should preferably pass in a NULL gdbarch. (Certainly preferable to using some random arch from target_gdbarch or get_current_arch ...) As an aside, I'm not really happy with the use of current_language in lookup_symbol. Ideally, this is one more of those globals we'd really better get rid of. (In particular, since many callers actually do know which language they're operating on.) I'd prefer an interface where lookup_symbol uses language_unknown, i.e. does not perform any language-specific lookup (which should actually be fine for many of the current users). If you do want do perform language-specific lookup, you should pass in the language explicitly. (Just mentioning this now since you're already going through all users :-)) > This patch also adds some const-correctness to gdbarch.*. > [It'll get split out of course.] This is just for the variable accessors and predicates, right? I guess that makes sense. Not sure if it really makes much of a difference since most callers will still need to call one of the other routines ... Bye, Ulrich -- Dr. Ulrich Weigand GNU/Linux compilers and toolchain Ulrich.Weigand@de.ibm.com