Re: [patch] PR python/10807 API for macros.

[Phil Muldoon <pmuldoon@redhat.com>]

Matt Rice <ratmice@gmail.com> writes:


>> What do you mean by inconsistent memory management?  Can you expand/explain
>> this.  Do macro definitions have a life-cycle in GDB?
>
> depends on the source of the macro, xmalloc directly for user-defined,
> or the macro table's obstack.
>
> from macroscope.c:
> /* A table of user-defined macros.  Unlike the macro tables used for
>    symtabs, this one uses xmalloc for all its allocation, not an
>    obstack, and it doesn't bcache anything; it just xmallocs things.  So
>    it's perfectly possible to remove things from this, or redefine
>    things.  */
>
> I'm second guessing myself that caching the macro_source file is safe though.

Is there life-cycle management for macros?  (See py-symbol.c for
life-cycle management of symbols).  If so, we should invalidate (but keep
around) the macro Python object, but run a validity routine to check
that the macro exists.

If not, then I am not sure.  If you cache the macro_source file, do you
keep it around forever?  I am also unsure if it is ok to do this.

>> Same as above, re failure.  Why do you need a custom hash function?
>
> needed the hash function for storing in sets
> for the same reason that set([{"a" : "b"}]) doesn't work
> Traceback (most recent call last):
>   File "<stdin>", line 1, in <module>
> TypeError: unhashable type: 'dict'
>
> mostly I was just using it in the tests,
> the only use case I can imagine is once we have a symtab_and_line
> macros() function
> you could use it to find the difference between 2 'macro scopes'

That's totally fine, I just missed the usage in the testsuite.

>> These comments are somewhat complex, and really follow the three paragraph
>> comments belows. So take them as one big comment.
>>
...

>> Normally, a function that does not return a Python object has to
>> deal with stack printing/error detection internally. Because this is not
>> a Python function (IE returning NULL signifies an error, you have to
>> check and deal with the error here)  this is done with
>> gdbpy_print_stack.  However ....

...

>> I'm really puzzled what to do here.  I'm assuming the iterator won't
>> stop because the helper function has encountered errors (at least, the
>> function pointer prototype is a void return).  Because macro_for_each is
>> a GDB iterator function that calls (through a function pointer and other
>> GCC helpers) add_macro_to_list, which itself calls Python functions,
>> each iteration can raise a Python error.  If we print and cope with the
>> error for each iteration in the helper as suggested above, the user will
>> see each exception and no exception data is overwritten.  Also there
>> seems (to me, at least) no way to abort the iterator earlier.
>>
>> OTOH we want to make sure that this function returns correctly,
>> according to how Python functions should.  So we SHOULD return NULL here
>> if there were errors, but if we do what I suggested above, every
>> exception will already be printed and cleared.  So returning NULL here
>> will cause Python to complain.  But I also don't want previous iteration
>> exceptions overwritten either. Maybe your way is right in that we only
>> report the last iterations exception.  Or maybe we should construct our
>> own exception and return that.  I do not know.  I'd appreciate
>> comments here from the maintainers.
>
> another approach may be to use a 'macro iterator' method which calls a
> python function with the converted arguments.  I initially had
> macro_object just copy everything into a python object.  but the
> macros() implementations used far too much memory (in a hello world)
> building a full list with each  macro containing an include_trail,

> with a macro iterator we wouldn't return a list, just call the
> callback for each macro, it'd mean error handling would have to happen
> in said python callback function, but we'd be free of any concerns
> with caching the macro_source_file, otoh memory size probably won't go
> away it just means we have punted said concerns to the user, if they
> try to store it in a list, they'd probably run into the same thing.

I think you and I are violently agreeing, but saying for effect
anyway. You might also call the iterator and store them temporarily in a
non-Python way (VEC, or something that suits your purpose).  Then as a
second-pass, convert them to the Python object, error-checking as
appropriate?

Cheers,

Phil