[PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.

[Duncan P. N. Exon Smith]

> On 2015 Feb 25, at 19:43, Romanova, Katya <Katya_Romanova at playstation.sony.com> wrote:
> 
> Hi,
> I'm having technical problems with Phabricator. Per Duncun request, I attached a patch for the review http://reviews.llvm.org/D7895
> Thanks!
> Katya.

I'm not the right person to review the high-level, but I have a
minor comment below.

> Index: lib/CodeGen/AsmPrinter/DwarfUnit.cpp
> ===================================================================
> --- lib/CodeGen/AsmPrinter/DwarfUnit.cpp	(revision 230313)
> +++ lib/CodeGen/AsmPrinter/DwarfUnit.cpp	(working copy)
> @@ -1207,12 +1207,20 @@
>    DIE &NDie = createAndAddDIE(dwarf::DW_TAG_namespace, *ContextDIE, NS);
>  
>    StringRef Name = NS.getName();
> -  if (!Name.empty())
> +  bool Anonymous = Name.empty();

Seems a little funny to me to have a variable for this.  `Name.empty()`
should be cheap enough, and the string in the `else` case already
documents the meaning.

However, if you do go with a variable, something like `IsAnonymous`
matches the coding standards better.

Maybe a better option would be to add a `DINamespace::isAnonymous()`
accessor that does the check, and just use it here.  (Switching to the
accessor should be a separate commit though.)

> +  if (!Anonymous)
>      addString(NDie, dwarf::DW_AT_name, NS.getName());
>    else
>      Name = "(anonymous namespace)";
>    DD->addAccelNamespace(Name, NDie);
>    addGlobalName(Name, NDie, NS.getContext());
> +  if (Anonymous) { 
> +    // For anonymous namespaces, add a DW_TAG_imported_module tag
> +    // containing an DW_AT_import attribute with the reference to this
> +    // namespace entry. 
> +    DIE &IDie = createAndAddDIE(dwarf::DW_TAG_imported_module, *ContextDIE);
> +    addDIEEntry(IDie, dwarf::DW_AT_import, NDie);
> +  }
>    addSourceLine(NDie, NS);
>    return &NDie;
>  }

> 
> 
> -----Original Message-----
> From: Duncan P. N. Exon Smith [mailto:dexonsmith at apple.com] 
> Sent: Wednesday, February 25, 2015 7:35 PM
> To: reviews+D7895+public+7827be49c0b04087 at reviews.llvm.org
> Cc: Romanova, Katya; Robinson, Paul; David Blaikie; Eric Christopher; llvm-commits at cs.uiuc.edu
> Subject: Re: [PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.
> 
> 
>> On 2015 Feb 25, at 19:19, Katya Romanova <Katya_Romanova at playstation.sony.com> wrote:
>> 
>> I have already created a review for this bug a few hours ago, but it didn't reach the full audience. I didn't add llvm--dev at first when I created the review. Apparently, adding it later doesn't have any effect and the review wasn't sent out to the mailing list. I abandoned the old review and created this one.
> 
> Sadly, there's still no patch on the list.  I'm not sure what you need to do to make phabricator post it for you.
> 
> Feel free to go "low-tech" and just attach the patch to an email (maybe in reply to this one?).
> 
> (BTW, Phabricator isn't required for reviews; it's just a tool that some find useful (IIUC because downloading/applying/editing patches is difficult in some environments?).)
> 
>> Here are a couple of comments that were added to the old review by the people who were specified as the reviewers:
>> 
>> from David Blaikie:
>> 
>>>> I'm not sure if this is really a bug in the debug info - seems like it might be reasonably treated as a bug in the debugger?
>> 
>>> 
>> 
>> 
>> from Paul Robinson:
>> 
>>>> Our debugger guys get pretty pedantic about interpreting the DWARF spec. I know GDB will auto-import info from an anonymous namespace, but:
>> 
>>> 
>> 
>>>> 
>> 
>>> 
>> 
>>>> the size cost here is really minimal (what, 5 or 6 bytes per 
>>>> anonymous namespace scope);
>> 
>>> 
>> 
>>>> they claimed that deciding whether to skip the namespace based on 
>>>> the name would be a problem for them;
>> 
>>> 
>> 
>>>> the compiler implementation is obviously trivial;
>> 
>>> 
>> 
>> 
>> 
>> 
>>>> so I didn't really feel motivated to argue the point.
>> 
>>> 
>> 
>> 
>> 
>> http://reviews.llvm.org/D7895
>> 
>> EMAIL PREFERENCES
>> http://reviews.llvm.org/settings/panel/emailpreferences/
>> 
>> 
>> 
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> 
> <anon2.txt>