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: Pierre-Marie de Rodat <derodat at adacore dot com>
- To: Pedro Alves <palves at redhat dot com>, GDB Patches <gdb-patches at sourceware dot org>
- Date: Tue, 09 Jun 2015 23:46:24 +0200
- 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>
Pedro,
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. ;-)
Thanks for the update, and sorry about the delay.
No problem. I probably won't be able to resume my work on this until
July, so more delay is to be expected. :-/ The good news is that then,
my GCC patches to fix the corresponding DWARF may be integrated. :-)
This overall looks very reasonable to me. It's fine with me to
let the core changes in, and address Python API issues separately.
Understood, thanks.
Add:
/* See block.h. */
I will.
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.
Will double-check and fix.
+ VAR_BLOCK is needed there's a possibility for VAR to be outside FRAME.
I think an "if" is missing after "needed".
Absolutely.
- val = read_var_value (var, frame);
+ /* READ_VAR_VALUE needs a block in order to deal with non-local
+ references (i.e. to handlee nested functions). In this context, we
typo "handle". Double space after period.
Will fix.
+# Please email any bugs, comments, and/or additions to this file to:
+# bug-gdb@gnu.org
It no longer makes sense to add this email address to tests. Please
drop it (here and elsewhere).
Sure.
+void iter_str (const char *str, void (*callback) (char c))
Could you make this follow GNU formatting? That is:
void
iter_str (const char *str, void (*callback) (char c))
Here and elsewhere.
I will.
+
+# Check we get correct values for both local and non-local variable references.
+
+# Note that in order to get the following test passing, one has to use a
+# patched GCC: see <https://gcc.gnu.org/bugzilla/show_bug.cgi?id=53927>.
+gdb_test "print first" "1"
+gdb_test "print parent_first" "1"
Please make this XFAIL instead of FAIL with unpatched GCC.
Otherwise looks good to me.
Ok. Thank you again for the review!
--
Pierre-Marie de Rodat