[PATCH] Anonymous namespaces are missing import DW_TAG_imported_module.

[Robinson, Paul]

> -----Original Message-----
> From: llvm-commits-bounces at cs.uiuc.edu [mailto:llvm-commits-
> bounces at cs.uiuc.edu] On Behalf Of Frédéric Riss
> Sent: Thursday, February 26, 2015 1:43 PM
> To: Romanova, Katya
> Cc: llvm-commits at cs.uiuc.edu;
> reviews+D7878+public+0975bde262e84f91 at reviews.llvm.org
> Subject: Re: [PATCH] Anonymous namespaces are missing import
> DW_TAG_imported_module.
> 
> 
> > On Feb 26, 2015, at 12:46 PM, Romanova, Katya
> <Katya_Romanova at playstation.sony.com> wrote:
> >
> > Hi,
> >
> > Our debugger guys argument is  that the debugger just follows the rules
> laid out by the debug information. If there is no import, the debugger
> will not import it. There is nothing in the  DWARF spec that clearly
> states an empty name is a magic import.
> 
> There is nothing in the dwarf spec that specifies that namespaces should
> be separated by ‘::’ in qualified names, yet the debugger has to know it
> :-) 

Only if the debugger requires you to manually enter qualified identifiers
in imitation C++ syntax.  Some do, but I've also used a GUI drill-down 
interface where separator syntax was totally irrelevant.

COBOL (he said, waxing reminiscent at the slightest provocation) lets
you omit the names of parent structure levels as long as the final name
is unambiguous. (And if some omitted level is an array, no problem--just
tack the subscript on wherever it's convenient.)  This is a major pain
for a command-line oriented debugger used to less forgiving languages like 
C++. ;-)  But with a GUI, people kind of expected to have to drill down 
every level, which had the advantage that the debugger didn't have to be 
as language-sensitive.

[I used to own the COBOL compiler for HP NonStop, and the debugger had
a GUI client.  Just to fill in the context there.]

So (returning to relevance) encoding that particular bit of C++ oddness
into a DWARF directive makes a certain amount of sense to me.

And, as David points out and I had forgotten, GCC does it that way too.

> My point simply was that the debugger has to have some knowledge about
> language semantics, and all the debuggers we know about already get this
> right without help from the compiler. 

So, now your knowledge of debuggers has increased? :-)

> We could also emit a DW_AT_name
> pointing to “(anonymous namespace)”, but we don’t and debuggers synthesize
> this themselves. There is a line somewhere between the language semantics
> encoded in the Dwarf and those implicitly known by the debugger. I guess
> this patch is near this line, I can’t decide on which side it stands.
> 
> > Personally I don't have a strong opinion about this issue. It was very
> simple to do in the compiler and it hardly increased size of the DWARF
> info. Apparently, it was much tougher to fix this in the debugger.
> >
> > If it is such a nuisance, maybe I could add an option and set it to
> false to all of the platforms, except PS4?
> 
> The added complexity wouldn’t make sense IMHO.
> 
> > Or I could follow Fred's suggestion and add DW_AT_artificial attribute?
> 
> Well, there is no ‘using’ or anything in the source, thus adding a
> DW_AT_artificial seems sound to me.

Yes, DW_AT_artificial on the DW_TAG_imported_module would be correct here.
--paulr

> 
> >>> However I don’t think the patch will break any debugger (although that
> might be worth testing).
> >
> > I could manually test GDB.
> > Fred, which other debuggers do you suggest to test to make sure that
> this patch didn't break them? Are there any existing debugger tests in
> LLVM?
> 
> Not that I expect any issue, but we care about lldb and gdb.
> 
> Fred.
> 
> > Thanks!
> > Katya
> >
> > -----Original Message-----
> > From: Frédéric Riss [mailto:friss at apple.com]
> > Sent: Thursday, February 26, 2015 9:46 AM
> > To: reviews+D7878+public+0975bde262e84f91 at reviews.llvm.org
> > Cc: Romanova, Katya; echristo at gmail.com; dblaikie at gmail.com; llvm-
> commits at cs.uiuc.edu
> > Subject: Re: [PATCH] Anonymous namespaces are missing import
> DW_TAG_imported_module.
> >
> >
> >> On Feb 25, 2015, at 2:25 PM, Paul Robinson
> <Paul_Robinson at playstation.sony.com> wrote:
> >>
> >> 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.
> >
> > Basically your debugger guys told you this could be described by
> standard constructs, so there is no point in adding extra logic to the
> debugger to handle it. IMHO this is not a very strong argument, because
> DWARF tries to be language agnostic, and thus every debugger needs to have
> some layer that adds language semantics to the raw debug information (for
> example, anonymous unions pose a very similar problem).
> >
> > However I don’t think the patch will break any debugger (although that
> might be worth testing). I know that it will have some impact on Darwin’s
> dsymutil though, because it considers the targets of every ‘import’
> directive as required when it decides which DIEs are needed in the final
> link. If the patch is going in, I think it would be nice to have a
> DW_AT_artificial in the importing DIE (which shouldn’t add any size to the
> debug info expect for one additional abbreviation).
> >
> > Fred
> >
> >> http://reviews.llvm.org/D7878
> >>
> >> 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
> >
> 
> 
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits