echristo 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; ---------------- probinson wrote: > 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. I saw. If you want that to be a required element of the spec then we need to change multiple wordings in the DWARF spec. I think the right place to bring that up is with the committee. The example is perfectly valid DWARF with the import, it's just unnecessary.
http://reviews.llvm.org/D15881 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
