This is the mail archive of the
gdb-patches@sourceware.org
mailing list for the GDB project.
Re: [PATCH] Add proper handling for non-local references in nested functions
- From: Doug Evans <xdje42 at gmail dot com>
- To: Pierre-Marie de Rodat <derodat at adacore dot com>
- Cc: Pedro Alves <palves at redhat dot com>, GDB Patches <gdb-patches at sourceware dot org>
- Date: Wed, 22 Jul 2015 07:26:02 -0700
- Subject: Re: [PATCH] Add proper handling for non-local references in nested functions
- Authentication-results: sourceware.org; auth=none
- References: <54F47563 dot 4050103 at adacore dot com> <54FF0D05 dot 70907 at redhat dot com> <550C1170 dot 9070208 at adacore dot com> <55685B60 dot 3000004 at redhat dot com> <55775EB0 dot 4080701 at adacore dot com> <55AF5F7E dot 5000600 at adacore dot com>
On Wed, Jul 22, 2015 at 2:16 AM, Pierre-Marie de Rodat
<derodat@adacore.com> wrote:
> On 06/09/2015 11:46 PM, Pierre-Marie de Rodat wrote:
>>
>> On 05/29/2015 02:28 PM, Pedro Alves wrote:
>>>>
>>>> This would look cleaner indeed. It's a big change itself though so if
>>>> most consider this as a good idea I don't mind doing it... although it
>>>> would be for another commit!
>>>
>>>
>>> I would think it great if someone did that. :-)
>>
>>
>> Okay... I may give it a try, then. ;-)
>
>
> Here it is! <https://sourceware.org/ml/gdb-patches/2015-07/msg00607.html> I
> just rebased my work on non-local references on top of this cleanup and
> performed the changes you asked me to do. Just a question:
>
>>> It'd be great if you could skim over the patch add any missing
>>> function intro comments. You've already done a good job at that,
>>> I think only here and there missed it.
>
>
> What I usually do is to put comments in front of function definitions and
> leave function declarations without them in the header. It's generally what
> I observe in the sources, but since sometimes the documentation is in the
> header file, it happens that I do the same: I try to stay consistent with
> nearby code. ;-) Please tell me if you want me to do something different.
>
> Regtested again on x86_64-linux: no regression. Ok to push? Thank you!
Hi.
Ditto. Others may approve this, but give me a chance to review it too.
One thought that comes to mind when reading the patch is that
you introduce the term "static link", and it doesn't mean what
the casual reader will think it means.
E.g.,
+ This method is designed to work with static links (nested
functions
+ handling). Static links are function properties whose
evaluation return
+ the frame base address for the enclosing frame.
I think we need something less ambiguous / more clear.