Re: [PATCH 08/14] Remove symtab links from dwarf2_psymtab and dwarf2_per_cu_quick_data

Simon Marchi
2020-02-19T04:47:19Z

>> @@ -2332,9 +2328,9 @@ dw2_do_instantiate_symtab (struct dwarf2_per_cu_data *per_cu, bool skip_partial)
>> with the dwarf queue empty. */
>> dwarf2_queue_guard q_guard (dwarf2_per_objfile);
>>
>> - if (dwarf2_per_objfile->using_index
>> - ? per_cu->v.quick->compunit_symtab == NULL
>> - : (per_cu->v.psymtab == NULL || !per_cu->v.psymtab->readin))
>> + gdb::optional<compunit_symtab *> &symtab
>> + = dwarf2_per_objfile->unshareable->symtabs[per_cu->index];
>> + if (!symtab.has_value ())
>
> I noticed the above sequence gets repeated a lot throughout the code.
> Can we turn it into a function/method and reduce the duplication?

I was going to say the same thing, a getter on dwarf2_unshareable would be nice. Since this
is a new data type, we might as well design it right from the start.

>> @@ -82,6 +82,10 @@ struct dwarf2_unshareable
>> This is NULL if not allocated yet.
>> The mapping is done via (CU/TU + DIE offset) -> type. */
>> htab_up die_type_hash;
>> +
>> + /* Hold the corresponding compunit_symtab for each CU or TU. This
>> + is indexed by dwarf2_per_cu_data::index. */
>> + std::vector<gdb::optional<compunit_symtab *>> symtabs;

Having pointers in an optional gives us three states:

1- not instantiated
2- instantiated but NULL
3- instantiated and not NULL

Do we actually need the distinction between 1 and 2 here? If not, I think the optional
doesn't bring much and just makes the code more complicated.

Simon