On 16-05-19 17:37, Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-05-03 11:31:26 +0200]:

> This all sounds good.  I have a couple of small suggestions inline
> below...
> 
>>
>> ---
>>  gdb/dwarf2read.c | 49 +++++++++++++++++++++++++++++++++++++++----------
>>  1 file changed, 39 insertions(+), 10 deletions(-)
>>
>> diff --git a/gdb/dwarf2read.c b/gdb/dwarf2read.c
>> index b0bdecf96f..442b618f6e 100644
>> --- a/gdb/dwarf2read.c
>> +++ b/gdb/dwarf2read.c
>> @@ -1518,8 +1518,14 @@ static unsigned int peek_abbrev_code (bfd *, const gdb_byte *);
>>  static struct partial_die_info *load_partial_dies
>>    (const struct die_reader_specs *, const gdb_byte *, int);
>>  
>> -static struct partial_die_info *find_partial_die (sect_offset, int,
>> -						  struct dwarf2_cu *);
>> +struct cu_partial_die_info
>> +{
>> +  struct dwarf2_cu *cu;
>> +  struct partial_die_info *pdi;
>> +};
> 
> This needs at least a header comment describing its use, and ideally
> each field documented too.
> 

Done.

> I wonder though if you should also provide this with a 2 argument
> constructor, and delete the default constructor, like:
> 
>   /* blah blah blah...  */
> 
>   struct cu_partial_die_info
>   {
>     /* mumble.. */
>     struct dwarf2_cu *cu;
> 
>     /* mutter...  */
>     struct partial_die_info *pdi;
> 
>     cu_partial_die_info (struct dwarf2_cu *cu,
>   		       struct partial_die_info *pdi)
>       : cu (cu),
>         pdi (pdi)
>     { /* Nothing.  */ }
> 
>   private:
>     cu_partial_die_info () = delete;
>   };
> 
> This will lead to some obvious knock on changes in the rest of the
> code, which I think are probably improvements.
> 

I've tried this out, and the only effect was this type of changes:
...
-  struct cu_partial_die_info res;
+  struct cu_partial_die_info res (NULL, NULL);
...
So, I've left this out for now.

Committed as below.

Also ok for 8.3 branch?

Thanks,
- Tom