[PATCH] D15881: [DWARF] Omitting the explicit import of an anonymous namespace is a debugger-tuning decision, not a target decision.

[Paul Robinson via cfe-commits]

probinson added inline comments.

================
Comment at: lib/Frontend/CompilerInvocation.cpp:446-448
@@ -445,3 +445,5 @@
   Opts.DebugTypeExtRefs = Args.hasArg(OPT_dwarf_ext_refs);
-  Opts.DebugExplicitImport = Triple.isPS4CPU(); 
+  Opts.DebugExplicitImport =
+      Opts.getDebuggerTuning() != CodeGenOptions::DebuggerKindGDB &&
+      Opts.getDebuggerTuning() != CodeGenOptions::DebuggerKindLLDB;
 
----------------
echristo wrote:
> probinson wrote:
> > echristo wrote:
> > > probinson wrote:
> > > > echristo wrote:
> > > > > probinson wrote:
> > > > > > probinson wrote:
> > > > > > > echristo wrote:
> > > > > > > > probinson wrote:
> > > > > > > > > echristo wrote:
> > > > > > > > > > Why not just a positive for debugger tuning SCE?
> > > > > > > > > Because the default (i.e., no tuning specified) behavior should be to conform to the DWARF spec, which basically says you need the explicit import.  There's a new extra RUN line in the test, with no tuning specified, to verify this.
> > > > > > > > > GDB and LLDB are the oddballs here, they implement a special case for namespaces whose name meets certain criteria, and do something beyond what DWARF says to do.  So, the condition is written to express that.
> > > > > > > > > 
> > > > > > > > I don't necessarily agree with that interpretation on the explicit import - I did skim the thread, perhaps you could highlight what makes you think this?
> > > > > > > Basically, a namespace is a "context" for declarations, and the DWARF mechanism for making declarations from one context available in another context is with DW_TAG_imported_declaration and DW_TAG_imported_module.
> > > > > > > The C++ spec describes the behavior "as if" there was an explicit using directive, and DW_TAG_imported_module is the DWARF mechanism for describing a using directive.
> > > > > > > 
> > > > > > > The meaning of DWARF is determined by the DWARF spec, not the C++ spec, and the DWARF spec does not say there's anything special about a namespace that has no name.  There is a perfectly reasonable DWARF mechanism for getting the desired effect, so there's no reason for DWARF to make a special rule for an unnamed namespace.  Therefore, an anonymous namespace should be explicitly imported into the containing namespace. The explicit import would be marked artificial of course.
> > > > > > > 
> > > > > > Ping.  Have I missed something in the DWARF spec that makes you think my interpretation is incorrect? Wouldn't be the first time...
> > > > > I don't have anything to add to the reasoning the David has given you. We both agree and let's just make this a positive tuning for SCE.
> > > > > 
> > > > Fine.  It'll be on just for SCE.
> > > > // Pedantic DWARF requires explicit import but only SCE insists.
> > > > 
> > > Please don't add that comment to this, I don't believe that it is valid or useful.
> > The DWARF committee disagrees with your validity opinion, but I will take the comment out.
> Bring it up on the list then.
I brought it up during the document review; see my ping comment from Jan 28.


http://reviews.llvm.org/D15881