Re: [PATCH 2/3] python: Add Progspace.objfiles method

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2018-09-12 18:01, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> 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