[PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not

[Richard Smith via cfe-commits]

rsmith added a comment.

I think this is fundamentally the right approach. Anonymous namespaces do inject a using directive, and we should not forget to pass it to the AST consumer.


================
Comment at: lib/CodeGen/CGDebugInfo.cpp:3263-3264
@@ +3262,4 @@
+  const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
+  if (!NSDecl->isAnonymousNamespace() || 
+      CGM.getTarget().getTriple().isPS4CPU()) { 
+    DBuilder.createImportedModule(
----------------
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.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:204
@@ +203,3 @@
+  
+ // Return NamespaceDecl + ImportDecl here.
+  return Actions.ConvertDeclToDeclGroup(NamespcDecl, 
----------------
This comment doesn't add anything and doesn't use the right name for either variable. Just remove it?

================
Comment at: lib/Parse/ParseDeclCXX.cpp:235
@@ -230,2 +234,3 @@
   ParseScope NamespaceScope(this, Scope::DeclScope);
+  UsingDirectiveDecl *ImplicitUsingDirectiveDecl = NULL;
   Decl *NamespcDecl =
----------------
Use `nullptr`, not `NULL`.

================
Comment at: lib/Parse/ParseDeclCXX.cpp:241
@@ -236,2 +240,3 @@
+                                   attrs.getList(), ImplicitUsingDirectiveDecl);
 
   ParseInnerNamespace(IdentLoc, Ident, NamespaceLoc, ++index, InlineLoc,
----------------
Add an assert here:

  assert(!ImplicitUsingDirectiveDecl && "nested namespace definition cannot define anonymous namespace");

================
Comment at: lib/Sema/SemaDeclCXX.cpp:7189
@@ -7189,1 +7188,3 @@
+                                   AttributeList *AttrList,
+                                   UsingDirectiveDecl *& UD) {
   SourceLocation StartLoc = InlineLoc.isValid() ? InlineLoc : NamespaceLoc;
----------------
No space after `&`.


http://reviews.llvm.org/D12624