r302840 - Module Debug Info: Emit namespaced C++ forward decls in the correct module.

Adrian Prantl via cfe-commits cfe-commits at lists.llvm.org
Thu May 11 17:05:39 PDT 2017


> On May 11, 2017, at 4:43 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On 11 May 2017 at 15:59, Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> Author: adrian
> Date: Thu May 11 17:59:19 2017
> New Revision: 302840
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=302840&view=rev
> Log:
> Module Debug Info: Emit namespaced C++ forward decls in the correct module.
> 
> The AST merges NamespaceDecls, but for module debug info it is
> important to put a namespace decl (or rather its children) into the
> correct (sub-)module, so we need to use the parent module of the decl
> that triggered this namespace to be serialized as a second key when
> looking up DINamespace nodes.
> 
> I don't think that's quite right; the AST doesn't merge NamespaceDecls. It looks like the issue is that we're mapping to the semantic DeclContext (which will be the canonical declaration of the namespace and could be from a different module) rather than the lexical DeclContext (the enclosing one, in the same module)... and then getOrCreateNameSpace is again mapping to the canonical declaration before calling getDeclContextDescriptor.

Thanks for pointing this out! It looks like I can simplify the code a lot if I can pass the lexical DeclContext to getOrCreateNamespace and then delegate the uniquing of the DINamespaces to DIBuilder (or rather MDNode), which will work now that DINamespaces don't carry line and file any more.

I see that there is Decl::getLexicalContext(), so this should be doable.

-- adrian
>  
> rdar://problem/29339538
> 
> Added:
>     cfe/trunk/test/Modules/DebugInfoNamespace.cpp
>     cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/
>     cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/A.h
>     cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/B.h
>     cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/module.modulemap
> Modified:
>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>     cfe/trunk/lib/CodeGen/CGDebugInfo.h
> 
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=302840&r1=302839&r2=302840&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Thu May 11 17:59:19 2017
> @@ -208,8 +208,10 @@ llvm::DIScope *CGDebugInfo::getContextDe
>    }
> 
>    // Check namespace.
> -  if (const auto *NSDecl = dyn_cast<NamespaceDecl>(Context))
> -    return getOrCreateNameSpace(NSDecl);
> +  if (const auto *NSDecl = dyn_cast<NamespaceDecl>(Context)) {
> +    auto *ParentModule = dyn_cast<llvm::DIModule>(Default);
> +    return getOrCreateNamespace(NSDecl, ParentModule);
> +  }
> 
>    if (const auto *RDecl = dyn_cast<RecordDecl>(Context))
>      if (!RDecl->isDependentType())
> @@ -2861,7 +2863,7 @@ void CGDebugInfo::collectFunctionDeclPro
>    if (DebugKind >= codegenoptions::LimitedDebugInfo) {
>      if (const NamespaceDecl *NSDecl =
>          dyn_cast_or_null<NamespaceDecl>(FD->getDeclContext()))
> -      FDContext = getOrCreateNameSpace(NSDecl);
> +    FDContext = getOrCreateNamespace(NSDecl, getParentModuleOrNull(FD));
>      else if (const RecordDecl *RDecl =
>               dyn_cast_or_null<RecordDecl>(FD->getDeclContext())) {
>        llvm::DIScope *Mod = getParentModuleOrNull(RDecl);
> @@ -3961,7 +3963,7 @@ void CGDebugInfo::EmitUsingDirective(con
>        CGM.getCodeGenOpts().DebugExplicitImport) {
>      DBuilder.createImportedModule(
>          getCurrentContextDescriptor(cast<Decl>(UD.getDeclContext())),
> -        getOrCreateNameSpace(NSDecl),
> +        getOrCreateNamespace(NSDecl, getParentModuleOrNull(&UD)),
>          getLineNumber(UD.getLocation()));
>    }
>  }
> @@ -4021,23 +4023,32 @@ CGDebugInfo::EmitNamespaceAlias(const Na
>    else
>      R = DBuilder.createImportedDeclaration(
>          getCurrentContextDescriptor(cast<Decl>(NA.getDeclContext())),
> -        getOrCreateNameSpace(cast<NamespaceDecl>(NA.getAliasedNamespace())),
> +        getOrCreateNamespace(cast<NamespaceDecl>(NA.getAliasedNamespace()),
> +                             getParentModuleOrNull(&NA)),
>          getLineNumber(NA.getLocation()), NA.getName());
>    VH.reset(R);
>    return R;
>  }
> 
>  llvm::DINamespace *
> -CGDebugInfo::getOrCreateNameSpace(const NamespaceDecl *NSDecl) {
> +CGDebugInfo::getOrCreateNamespace(const NamespaceDecl *NSDecl,
> +                                  llvm::DIModule *ParentModule) {
>    NSDecl = NSDecl->getCanonicalDecl();
> -  auto I = NameSpaceCache.find(NSDecl);
> -  if (I != NameSpaceCache.end())
> +  // The AST merges NamespaceDecls, but for module debug info it is important to
> +  // put a namespace decl (or rather its children) into the correct
> +  // (sub-)module, so use the parent module of the decl that triggered this
> +  // namespace to be serialized as a second key.
> +  NamespaceKey Key = {NSDecl, ParentModule};
> +  auto I = NamespaceCache.find(Key);
> +  if (I != NamespaceCache.end())
>      return cast<llvm::DINamespace>(I->second);
> 
>    llvm::DIScope *Context = getDeclContextDescriptor(NSDecl);
> -  llvm::DINamespace *NS =
> -      DBuilder.createNameSpace(Context, NSDecl->getName(), NSDecl->isInline());
> -  NameSpaceCache[NSDecl].reset(NS);
> +  // Don't trust the context if it is a DIModule (see comment above).
> +  llvm::DINamespace *NS = DBuilder.createNameSpace(
> +      isa<llvm::DIModule>(Context) ? ParentModule : Context, NSDecl->getName(),
> +      NSDecl->isInline());
> +  NamespaceCache[Key].reset(NS);
>    return NS;
>  }
> 
> 
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.h?rev=302840&r1=302839&r2=302840&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.h (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.h Thu May 11 17:59:19 2017
> @@ -125,7 +125,8 @@ class CGDebugInfo {
>    /// Cache declarations relevant to DW_TAG_imported_declarations (C++
>    /// using declarations) that aren't covered by other more specific caches.
>    llvm::DenseMap<const Decl *, llvm::TrackingMDRef> DeclCache;
> -  llvm::DenseMap<const NamespaceDecl *, llvm::TrackingMDRef> NameSpaceCache;
> +  typedef std::pair<const NamespaceDecl *, const llvm::DIModule *> NamespaceKey;
> +  llvm::DenseMap<NamespaceKey, llvm::TrackingMDRef> NamespaceCache;
>    llvm::DenseMap<const NamespaceAliasDecl *, llvm::TrackingMDRef>
>        NamespaceAliasCache;
>    llvm::DenseMap<const Decl *, llvm::TypedTrackingMDRef<llvm::DIDerivedType>>
> @@ -194,8 +195,15 @@ class CGDebugInfo {
>    getOrCreateFunctionType(const Decl *D, QualType FnType, llvm::DIFile *F);
>    /// \return debug info descriptor for vtable.
>    llvm::DIType *getOrCreateVTablePtrType(llvm::DIFile *F);
> +
> +  /// \return namespace descriptor for the given namespace decl.
> +  ///
>    /// \return namespace descriptor for the given namespace decl.
> -  llvm::DINamespace *getOrCreateNameSpace(const NamespaceDecl *N);
> +  /// \param ParentModule  The parent module (or nullptr) of this particular
> +  ///                      namespace decl. This needs to be passed in because
> +  ///                      the AST merges namespace decls.
> +  llvm::DINamespace *getOrCreateNamespace(const NamespaceDecl *N,
> +                                          llvm::DIModule *ParentModule);
>    llvm::DIType *CreatePointerLikeType(llvm::dwarf::Tag Tag, const Type *Ty,
>                                        QualType PointeeTy, llvm::DIFile *F);
>    llvm::DIType *getOrCreateStructPtrType(StringRef Name, llvm::DIType *&Cache);
> 
> Added: cfe/trunk/test/Modules/DebugInfoNamespace.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/DebugInfoNamespace.cpp?rev=302840&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/DebugInfoNamespace.cpp (added)
> +++ cfe/trunk/test/Modules/DebugInfoNamespace.cpp Thu May 11 17:59:19 2017
> @@ -0,0 +1,19 @@
> +// RUN: rm -rf %t
> +// RUN: %clang_cc1 -x objective-c++ -std=c++11 -debug-info-kind=standalone \
> +// RUN:     -dwarf-ext-refs -fmodules \
> +// RUN:     -fmodule-format=obj -fimplicit-module-maps \
> +// RUN:     -triple %itanium_abi_triple -fmodules-cache-path=%t \
> +// RUN:     %s -I %S/Inputs/DebugInfoNamespace -I %t -emit-llvm -o - \
> +// RUN:     |  FileCheck %s
> +
> +#include "A.h"
> +#include "B.h"
> +using namespace N;
> +B b;
> +
> +// Verify that the forward decl of B is in module B.
> +//
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "B",
> +// CHECK-SAME:             scope: ![[N:[0-9]+]]
> +// CHECK: ![[N]] = !DINamespace(name: "N", scope: ![[B:[0-9]+]])
> +// CHECK: ![[B]] = !DIModule(scope: null, name: "B",
> 
> Added: cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/A.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/A.h?rev=302840&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/A.h (added)
> +++ cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/A.h Thu May 11 17:59:19 2017
> @@ -0,0 +1,3 @@
> +namespace N {
> +  struct A {};
> +}
> 
> Added: cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/B.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/B.h?rev=302840&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/B.h (added)
> +++ cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/B.h Thu May 11 17:59:19 2017
> @@ -0,0 +1,3 @@
> +namespace N {
> +  struct B {};
> +}
> 
> Added: cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/module.modulemap
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/module.modulemap?rev=302840&view=auto
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/module.modulemap (added)
> +++ cfe/trunk/test/Modules/Inputs/DebugInfoNamespace/module.modulemap Thu May 11 17:59:19 2017
> @@ -0,0 +1,8 @@
> +module A {
> +  header "A.h"
> +  export *
> +}
> +module B {
> +  header "B.h"
> +  export *
> +}
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> 



More information about the cfe-commits mailing list