Replace the block_found global with explicit data-flow
Commit Message
On 08/01/2015 06:04 PM, Luis Machado wrote:
>> + sym = lookup_symbol (MDEBUG_EFI_SYMBOL_NAME, b, LABEL_DOMAIN,
>> + 0).symbol;
>> }
>
> It is probably a matter of personal taste, but i find it easier and
> cleaner to get the result of lookup_symbol (...) assigned to a variable
> and then have the code access its 'symbol' field as opposed to chaining
> things like the above.
>
> The current codebase doesn't show many occurrences like the above.
Understood: here's an updated patch introducing a temporary to do this.
Thank you!
Comments
On 08/01/2015 01:25 PM, Pierre-Marie de Rodat wrote:
> On 08/01/2015 06:04 PM, Luis Machado wrote:
>>> + sym = lookup_symbol (MDEBUG_EFI_SYMBOL_NAME, b, LABEL_DOMAIN,
>>> + 0).symbol;
>>> }
>>
>> It is probably a matter of personal taste, but i find it easier and
>> cleaner to get the result of lookup_symbol (...) assigned to a variable
>> and then have the code access its 'symbol' field as opposed to chaining
>> things like the above.
>>
>> The current codebase doesn't show many occurrences like the above.
>
> Understood: here's an updated patch introducing a temporary to do this.
> Thank you!
>
Thanks!
I think you forgot to fix the other occurrences though.
On 08/01/2015 08:24 PM, Luis Machado wrote:
>> Understood: here's an updated patch introducing a temporary to do this.
>> Thank you!
>
> Thanks!
>
> I think you forgot to fix the other occurrences though.
Arg, sorry: I thought you were suggesting this to get rid of the weird
indentation behavior (hence my fix on the first occurence only).
At this point I wonder what to do: this patch indeed introduces some of
“foo(..).bar” constructs, but my previous one introduced a _lot_ more.
So my feeling is that the code will get bigger and less readable...
that's my personal taste anyway. ;-) That being said, I'm completely
open to settling on this style anyway, as long as we are aware of the
consequences.
On 08/01/2015 04:35 PM, Pierre-Marie de Rodat wrote:
> On 08/01/2015 08:24 PM, Luis Machado wrote:
>>> Understood: here's an updated patch introducing a temporary to do this.
>>> Thank you!
>>
>> Thanks!
>>
>> I think you forgot to fix the other occurrences though.
>
> Arg, sorry: I thought you were suggesting this to get rid of the weird
> indentation behavior (hence my fix on the first occurence only).
>
> At this point I wonder what to do: this patch indeed introduces some of
> “foo(..).bar” constructs, but my previous one introduced a _lot_ more.
> So my feeling is that the code will get bigger and less readable...
> that's my personal taste anyway. ;-) That being said, I'm completely
> open to settling on this style anyway, as long as we are aware of the
> consequences.
>
Indeed. I didn't review the original patch, so only this one caught my
attention. I agree it is not worth fixing this if a bunch of other
occurrences were added.
Maybe a new function should've been introduced to avoid having to keep
accessing that field, which would be both shorter and cleaner.
Then again, it is probably only costmetic. The move to C++ will
potentially introduce lots more occurrences in the future. :-)
From 426cecfa72c713442341a77a68b853c4a97e60d7 Mon Sep 17 00:00:00 2001
From: Pierre-Marie de Rodat <derodat@adacore.com>
Date: Sat, 1 Aug 2015 11:25:44 +0200
Subject: [PATCH] Complete the previous commit (block_found refactoring)
The previous commit (Replace the block_found global with explicit
data-flow) lacks updates in a couple of files because it was not tested
building GDB with --enable-targets=all... but buildbots did. This adds
the appropriate simple updates to fix buildbots.
gdb/
* alpha-mdebug-tdep.c (find_proc_desc): Update call to
lookup_symbol.
* ft32-tdep.c (ft32_skip_prologue): Likewise.
* moxie-tdep.c (moxie_skip_prologue): Likewise.
* mt-tdep.c (mt_skip_prologue): Likewise.
* xstormy16-tdep.c (xstormy16_skip_prologue): Likewise.
---
gdb/ChangeLog | 9 +++++++++
gdb/alpha-mdebug-tdep.c | 7 ++++++-
gdb/ft32-tdep.c | 2 +-
gdb/moxie-tdep.c | 2 +-
gdb/mt-tdep.c | 2 +-
gdb/xstormy16-tdep.c | 2 +-
6 files changed, 19 insertions(+), 5 deletions(-)
@@ -1,5 +1,14 @@
2015-08-01 Pierre-Marie de Rodat <derodat@adacore.com>
+ * alpha-mdebug-tdep.c (find_proc_desc): Update call to
+ lookup_symbol.
+ * ft32-tdep.c (ft32_skip_prologue): Likewise.
+ * moxie-tdep.c (moxie_skip_prologue): Likewise.
+ * mt-tdep.c (mt_skip_prologue): Likewise.
+ * xstormy16-tdep.c (xstormy16_skip_prologue): Likewise.
+
+2015-08-01 Pierre-Marie de Rodat <derodat@adacore.com>
+
* ada-exp.y (write_object_renaming): Replace struct
ada_symbol_info with struct block_symbol. Update field
references accordingly.
@@ -107,7 +107,12 @@ find_proc_desc (CORE_ADDR pc)
symbol reading. */
sym = NULL;
else
- sym = lookup_symbol (MDEBUG_EFI_SYMBOL_NAME, b, LABEL_DOMAIN, 0);
+ {
+ struct block_symbol bsym
+ = lookup_symbol (MDEBUG_EFI_SYMBOL_NAME, b, LABEL_DOMAIN, 0);
+
+ sym = bsym.symbol;
+ }
}
if (sym)
@@ -250,7 +250,7 @@ ft32_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
plg_end = ft32_analyze_prologue (func_addr,
func_end, &cache, gdbarch);
/* Found a function. */
- sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL);
+ sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL).symbol;
/* Don't use line number debug info for assembly source files. */
if ((sym != NULL) && SYMBOL_LANGUAGE (sym) != language_asm)
{
@@ -241,7 +241,7 @@ moxie_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
plg_end = moxie_analyze_prologue (func_addr,
func_end, &cache, gdbarch);
/* Found a function. */
- sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL);
+ sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL).symbol;
/* Don't use line number debug info for assembly source
files. */
if (sym && SYMBOL_LANGUAGE (sym) != language_asm)
@@ -415,7 +415,7 @@ mt_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
struct symbol *sym;
/* Found a function. */
- sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL);
+ sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL).symbol;
if (sym && SYMBOL_LANGUAGE (sym) != language_asm)
{
/* Don't use this trick for assembly source files. */
@@ -433,7 +433,7 @@ xstormy16_skip_prologue (struct gdbarch *gdbarch, CORE_ADDR pc)
return plg_end;
/* Found a function. */
- sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL);
+ sym = lookup_symbol (func_name, NULL, VAR_DOMAIN, NULL).symbol;
/* Don't use line number debug info for assembly source files. */
if (sym && SYMBOL_LANGUAGE (sym) != language_asm)
{
--
2.4.6