Re: [PATCH] [gdb/symtab] Factor out new_symbol_file_line

[Tom de Vries <tdevries@suse.de>]

On 4/22/26 6:51 PM, Tom Tromey wrote:
>>>>>> "Tom" == Tom de Vries <tdevries@suse.de> writes:
> 
> Tom> I was looking at a patch [1] modifying the DW_AT_decl_file / DW_AT_call_file
> Tom> handling of new_symbol, and noticed that:
> Tom> - the code is fairly nested, and
> Tom> - the patch adds another nesting layer.
> 
> Tom> Factor out function new_symbol_file_line out of new_symbol, handling both the
> Tom> DW_AT_decl_file / DW_AT_call_file and DW_AT_call_line / DW_AT_decl_line
> Tom> attributes.
> 
> Thanks for doing this.
> 
> I think new_symbol really needs to be split up, the way it is now is
> pretty bad, and your patch is a nice step in that direction.
>

Hi Tom,

Thanks for the review, pushed with the nit pointed out below fixed.

I don't know whether this is what you had in mind, but I've attempted a 
split up here ( 
https://sourceware.org/pipermail/gdb-patches/2026-April/226731.html ).

Thanks,
- Tom


> Tom> Having factored out the code, simplify it using return, and modernize it using
> Tom> bool and nullptr.
> 
> Tom> +  struct attribute *attr = nullptr;
> Tom> +
> Tom> +  bool inlined_func = (die->tag == DW_TAG_inlined_subroutine);
> Tom> +
> Tom> +  /* Handle DW_AT_call_line / DW_AT_decl_line.  */
> Tom> +  attr = dwarf2_attr (die,
> Tom> +		      inlined_func ? DW_AT_call_line : DW_AT_decl_line,
> Tom> +		      cu);
> 
> This line could be combined with the declaration and then the
> initialization removed like
> 
> attribute *attr = dwarf2_attr (...)
> 
> Approved-By: Tom Tromey <tom@tromey.com>
> 
> Tom