From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120662 invoked by alias); 12 Sep 2018 22:58:37 -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 120653 invoked by uid 89); 12 Sep 2018 22:58:36 -0000 Authentication-Results: sourceware.org; auth=none X-Spam-SWARE-Status: No, score=-2.4 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_PASS autolearn=ham version=3.3.2 spammy=sure! X-HELO: smtp.polymtl.ca Received: from smtp.polymtl.ca (HELO smtp.polymtl.ca) (132.207.4.11) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 12 Sep 2018 22:58:35 +0000 Received: from simark.ca (simark.ca [158.69.221.121]) (authenticated bits=0) by smtp.polymtl.ca (8.14.7/8.14.7) with ESMTP id w8CMwSKL031764 (version=TLSv1/SSLv3 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=NOT) for ; Wed, 12 Sep 2018 18:58:33 -0400 Received: by simark.ca (Postfix, from userid 112) id 998021E5A4; Wed, 12 Sep 2018 18:58:28 -0400 (EDT) Received: from simark.ca (localhost [127.0.0.1]) by simark.ca (Postfix) with ESMTP id C42FA1E4B5; Wed, 12 Sep 2018 18:58:27 -0400 (EDT) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Wed, 12 Sep 2018 22:58:00 -0000 From: Simon Marchi To: Tom Tromey Cc: Simon Marchi , gdb-patches@sourceware.org Subject: Re: [PATCH 2/3] python: Add Progspace.objfiles method In-Reply-To: <874leujrxz.fsf@tromey.com> References: <20180912193617.16523-1-simon.marchi@ericsson.com> <20180912193617.16523-2-simon.marchi@ericsson.com> <874leujrxz.fsf@tromey.com> Message-ID: <3d6d1484e08f5a40da317520443fb58d@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.3.6 X-IsSubscribed: yes X-SW-Source: 2018-09/txt/msg00386.txt.bz2 On 2018-09-12 18:01, Tom Tromey wrote: >>>>>> "Simon" == Simon Marchi writes: > > Simon> Question: > > Simon> When we try to access a property of an Inferior object that has > Simon> become invalid, for example, we raise an exception ("Inferior no > longer > Simon> exists."). When doing the same with a Progspace object, we > return None > Simon> (the only case for now is its filename property). For > Simon> Progspace.objfiles(), I made it return None too, but perhaps it > should > Simon> throw an exception instead? Especially that None is not > iterable, so > Simon> trying to do: > > Simon> for obj in pspace.objfiles(): > Simon> ... > > Simon> will fail horribly if we return None... so should I introduce a > macro > Simon> similar to INFPY_REQUIRE_VALID? > > There are two approaches to modeling gdb objects in the Python layer. > > One is taken by objects like Value whose lifetime can be arbitrarily > "extended". For these objects, Python simply holds a reference to the > underlying gdb object. > > The other approach is for objects whose lifetime can be controlled by > the user or other external (to Python) events. For example, a > breakpoint can be deleted by the user, leaving behind the > gdb.Breakpoint > representation. > > For these we have generally had the Python object keep a sort of weak > reference to the gdb object; when the gdb object is destroyed, the > Python wrapper enters a special invalid state. These objects have an > is_valid method; and generally all other methods and attributes throw > an > exception if the object is invalid -- but I think this is not a > hard-and-fast rule and can be broken where there is an obvious decent > non-exception result. > > In sum I think INFPY_REQUIRE_VALID is fine if you happen to need it at > some spot in the inferior wrapper. I wouldn't go out of my way to > avoid > it. This is already how the progspace object works (the weak reference thing), and I see you've added a PSPY_REQUIRE_VALID in your old patch. > Normally Python code has to know not to work with an invalid object > (and > anyway why would it want to); but I think in this case, I would be fine > with objfiles() returning an empty sequence. That is what my old patch > did, I would assume intentionally, though I don't recall. For consistency with the rest of the API, I would lean towards an exception. I will update the patch to do that. The existing "filename" property returns None in that case, unfortunately. > Finally, I have another ancient and unfinished series that adds a bunch > of methods to Inferior. If you're working in this area I can send it; > I'd be happy to rebase it. Sure! Thanks, Simon