From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 103650 invoked by alias); 1 Feb 2016 03:33:00 -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 103636 invoked by uid 89); 1 Feb 2016 03:32:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-0.8 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,RCVD_IN_DNSWL_NONE autolearn=no version=3.3.2 spammy=itll, it'll, Hx-languages-length:2827, buck X-HELO: rock.gnat.com Received: from rock.gnat.com (HELO rock.gnat.com) (205.232.38.15) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-SHA encrypted) ESMTPS; Mon, 01 Feb 2016 03:32:58 +0000 Received: from localhost (localhost.localdomain [127.0.0.1]) by filtered-rock.gnat.com (Postfix) with ESMTP id DE7FC1165AE; Sun, 31 Jan 2016 22:32:56 -0500 (EST) Received: from rock.gnat.com ([127.0.0.1]) by localhost (rock.gnat.com [127.0.0.1]) (amavisd-new, port 10024) with LMTP id B7-+PwkxxQsv; Sun, 31 Jan 2016 22:32:56 -0500 (EST) Received: from joel.gnat.com (localhost.localdomain [127.0.0.1]) by rock.gnat.com (Postfix) with ESMTP id 7E8AC1164A0; Sun, 31 Jan 2016 22:32:56 -0500 (EST) Received: by joel.gnat.com (Postfix, from userid 1000) id 594A8407D2; Mon, 1 Feb 2016 07:32:47 +0400 (RET) Date: Mon, 01 Feb 2016 03:33:00 -0000 From: Joel Brobecker To: Keith Seitz Cc: gdb-patches@sourceware.org Subject: Re: [PATCH] python/19506 -- gdb.Breakpoint address location regression Message-ID: <20160201033247.GI4008@adacore.com> References: <1453413926-24995-1-git-send-email-keiths@redhat.com> <20160126122256.GH5146@adacore.com> <56A81951.2030105@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <56A81951.2030105@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2016-02/txt/msg00003.txt.bz2 > [Apologies for waxing a little philosophical here...] [Apologies for triggering the discussion ;-)...] > For me, #1 really boils down to UI writers "passing the buck" down to > gdb's internals (via CLI generalisms). With the location API work, I > tried to make this layer a little more distinct/separate. UIs are > responsible for implementing their own representations of locations in a > way most consistent with their implementation specifics. [Example: using > 'gdb.Breakpoint("-source foo.c -line 3")' seems downright wrong to me.] Agreed. > In light of this, I am suggesting that instead of requiring > python/mi/guile/whatever to implement their own string_to_event_location > functions, that the current string_to_event_location be split into a > "legacy" portion (that deals with address, probe, and linespec > locations) and a newer (and separate) explicit-handling form (for the > CLI only). > > This leaves python/guile to implement an explicit location specification > in a manner more consistent with those interpreters, e.g., python could > use named arguments to gdb.Breakpoint: > > gdb.Breakpoint(source="foo.c", line="3") I agree that each extension language should have their own clean API for creating breakpoints using explicit locations. That's a lot cleaner. > This way we wouldn't "force" these languages to use the CLI's > argument/value paradigm. I've already done something similar for MI. [MI > is currently using string_to_event_location, which means it will > erroneously support the CLI's explicit syntax in addition to its own! > Bug! My bad! Have patch.] > > In short: > 1) Remove explicit locations from string_to_event_location. > Use this "new" function /everywhere/ legacy linespec support is > desired. In my working tree, I call this > string_to_event_location_basic. > 2) string_to_event_locations can now be refactored to do two things: > check for an explicit location OR call the _basic version to deal > with linespec, address, and probe locations. > > WDYT? [I can send a small patch series to address/clean up all of these, > if you think this is a reasonable resolution.] It sounds like a good idea, as I think it'll factorize the code. In the grand scheme of things, the current situation is not all that bad, though, so I wouldn't say that this absolutely needs to be done ahead of everything else. So, what I would suggest is first push the current patch if people agree with it. That way, we are covered for the release branch. And then, if you have some time to look at this enhancement before we release 7.11, then we can look at possibly backporting it. Did anyone review the patch? Typically, Doug is the one reviewing those patches, but I can take a look as well. Thanks, Keith! -- Joel