probinson added inline comments.
================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3263-3264
@@ +3262,4 @@
+ const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
+ if (!NSDecl->isAnonymousNamespace() ||
+ CGM.getTarget().getTriple().isPS4CPU()) {
+ DBuilder.createImportedModule(
----------------
rsmith wrote:
> probinson wrote:
> > rsmith wrote:
> > > I think we should do this unconditionally, to better match the source
> > > language semantics, but I'm curious what David, Eric, and other folks on
> > > the DWARF side think.
> > David (in previous discussions and review comments) has said he thinks it
> > is unnecessary as the debugger already must know so much about C++ to get
> > various things right, it might as well know that it has to implicitly
> > import the anonymous namespace contents. One example debugger UI allows
> > the user to type source-like syntax, and requires the debugger to apply
> > (for example) C++ parameter-type matching rules to distinguish between
> > overloaded functions. Compared to this, implicit imports are child's play.
> >
> > I believe Eric agrees with David; I don't remember whether Adrian said
> > anything in the previous iterations of this patch.
> >
> > I believe the explicit (although artificial) import is a good thing,
> > because it matches the source language semantics. I find an important
> > distinction between "which declarations are available in this scope" and
> > "how does the user disambiguate declarations in this scope." As a
> > counterpart to the above debugger UI example, I postulate a GUI drop-down
> > list of symbols available in-scope; this UI needs to know nothing about
> > language semantics and automatic imports, if the DWARF provides the correct
> > explicit import. This suggests to me that the DWARF should provide it.
> >
> > There's also the piddly detail that debuggers are not the only consumers of
> > DWARF information, and presenting the DWARF in a more
> > source-language-neutral way (i.e., with the explicit artificial import)
> > could be beneficial for those other consumers, who might not necessarily
> > want to learn language-specific scoping rules.
> >
> > No debugger will be thrown for a loop if it sees the explicit import;
> > however for some debuggers it would be redundant (because they implicitly
> > import the anonymous namespace already). There is a pretty trivial space
> > savings if it's omitted.
> >
> > Katya has mentioned the GCC and ICC precedent; in fairness I will say GCC
> > didn't used to emit this, and GDB tolerated that.
> >
> > Note that the DWARF standard does not tell us what to do; it merely tells
> > us how to emit the import, if we want to emit one. Whether we want to emit
> > one is up to us.
> >
> I've chatted to David about this offline, and he said largely similar things.
> It seems that different DWARF consumers will want and expect different things
> here, so (sadly) we should do different things depending on who we think will
> be consuming the DWARF.
>
> I'm fine keeping this conditional, but I don't think IR generation should be
> making this decision based on the triple, so I'd prefer it was phrased in a
> different way: add a CodeGenOpt for whether to emit imports for anonymous
> namespaces, and enable it for PS4 targets from the frontend.
I think including the explicit import is not actively harmful to any consumer,
or to object-file size; but if somebody insists on not having it, okay.
Assuming Katya is willing to invent the CodeGenOpt, I should add that not too
far down on my to-do list is plumbing the LLVM "debugger tuning" idea up
through Clang. We can make the tuning (rather than the triple) set the
CodeGenOpt when the time comes.
http://reviews.llvm.org/D12624
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
