On Tue, Sep 8, 2015 at 3:51 PM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:
> rsmith 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(
> ----------------
> 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.
>
This seems pretty fine-grained for a CodeGenOpt (not that I've looked
there, perhaps there are examples of similarly fine grained things already
there?)- I'm curious to understand the preference towards that rather than
perhaps the more general "Debugger tuning" sort of thing Paul's
implemented/could be pushed up here.
- Dave
>
>
> http://reviews.llvm.org/D12624
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
