Re: [PATCH v3] gdb: CTF support

[Simon Marchi <simon.marchi@polymtl.ca>]

On 2019-10-07 8:07 a.m., Andrew Burgess wrote:
> * Tom de Vries <tdevries@suse.de> [2019-10-07 11:33:12 +0200]:
> 
>> On 04-10-19 00:56, Weimin Pan wrote:
>>> +/* The routines that read and process fields/members of a C struct, union,
>>> +   or enumeration, pass lists of data member fields in an instance of a
>>> +   field_info structure. It is derived from dwarf2read.c.  */
>>> +
>>> +struct nextfield
>>> +{
>>> +  struct field field {};
>>> +};
>>> +
>>> +struct field_info
>>
>> Hi,
>>
>> not only is field_info derived from dwarf2read.c, it uses the same name
>> for the type.  This is a C++ One-Definition-Rule violation, which causes
>> most of the test-suite to start failing for me.
>>
>> What happens is that here:
>> ...
>>   if (die->child != NULL && ! die_is_declaration (die, cu))
>>     {
>>       struct field_info fi;
>>       std::vector<struct symbol *> template_args;
>> ...
>> the constructor for field_info is called, but it calls the constructor
>> for field_info defined in ctfread.c rather than dwarf2read.c.
> 
> Tom,
> 
> Thanks for tracking this down.  I had just run into the same issue.
> I've pushed the patch below which I believe fixes this issue.
> 
> Weimin,
> 
> Hopefully you're happy with this fix, I guess if you'd rather see an
> alternative solution then feel free to propose one.
> 
> Thanks,
> Andrew
> 
> ---
> 
> From b2caee6aaa78106d7ae3c46dda3a84a325e43a1d Mon Sep 17 00:00:00 2001
> From: Andrew Burgess <andrew.burgess@embecosm.com>
> Date: Mon, 7 Oct 2019 12:34:51 +0100
> Subject: [PATCH] gdb: Rename structures within ctfread.c
> 
> Commit:
> 
>   commit 30d1f0184953478d14641c495261afd06ebfabac
>   Date:   Mon Oct 7 00:46:52 2019 +0000
> 
>       gdb: CTF support
> 
> Introduces some structures with names that are already in use within
> GBB, this violates C++'s one-definition rule.  Specifically the
> structures 'nextfield' and 'field_info' are now defined in
> dwarf2read.c and ctfread.c.
> 
> This commit renames the new structures (in ctfread.c), adding a 'ctf_'
> prefix.  Maybe we should consider renaming the DWARF versions too in
> the future to avoid accidental conflicts.

Thanks guys for catching this.  Just in case you didn't already know,
another way to avoid these problems (if we apply it everywhere) is to
define everything that is local to a source file in an anonymous
namespace.  I don't really have the habit of doing it at the moment,
but maybe I should start to do it consistently.

Simon