> On Apr 25, 2016, at 5:34 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
>
> On Mon, Apr 25, 2016 at 1:52 PM, Adrian Prantl via cfe-commits
> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote:
> Author: adrian
> Date: Mon Apr 25 15:52:40 2016
> New Revision: 267464
>
> URL: http://llvm.org/viewvc/llvm-project?rev=267464&view=rev
> <http://llvm.org/viewvc/llvm-project?rev=267464&view=rev>
> Log:
> Module Debugging: Fix the condition for determining whether a template
> instantiation is in a module.
>
> This patch fixes the condition for determining whether the debug info for a
> template instantiation will exist in an imported clang module by:
>
> - checking whether the ClassTemplateSpecializationDecl is complete and
> - checking that the instantiation was in a module by looking at the first
> field.
>
> I also added a negative check to make sure that a typedef to a
> forward-declared
> template (with the definition outside of the module) is handled correctly.
>
> http://reviews.llvm.org/D19443 <http://reviews.llvm.org/D19443>
> rdar://problem/25553724
>
> Modified:
> cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> cfe/trunk/test/Modules/ExtDebugInfo.cpp
> cfe/trunk/test/Modules/Inputs/DebugCXX.h
> cfe/trunk/test/Modules/ModuleDebugInfo.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=267464&r1=267463&r2=267464&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=267464&r1=267463&r2=267464&view=diff>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Mon Apr 25 15:52:40 2016
> @@ -1514,12 +1514,28 @@ static bool hasExplicitMemberDefinition(
> return false;
> }
>
> +/// Does a type definition exist in an imported clang module?
> +static bool isDefinedInClangModule(const RecordDecl *RD) {
> + if (!RD->isFromASTFile())
> + return false;
> + if (!RD->getDefinition())
> + return false;
> + if (!RD->isExternallyVisible() && RD->getName().empty())
> + return false;
> + if (auto *CTSD = dyn_cast<ClassTemplateSpecializationDecl>(RD)) {
>
> The same issue also affects member classes of class template specializations
> (you can detect them with RD->getInstantiatedFromMemberClass(), or detect all
> the relevant cases at once with RD->getTemplateSpecializationKind()).
I added a testcase for this r267612, but I couldn’t reproduce the situation
that you were describing here. Do you have an example of what you had in mind?
>
> + if (!CTSD->isCompleteDefinition())
> + return false;
>
> You already checked this a few lines above. Actually, the two checks do
> different things depending on whether RD or some other declaration is the
> definition; did you really mean to treat a (non-template) class that's
> defined locally but forward-declared within a module as being defined in that
> module? (It might make more sense to map to the definition upfront and do all
> your queries on that if that's what you intend to check.)
Thanks! I changed this in r267611 to always pass RD->getDefinition() into
isDefinedInClangModule. Does this make the check for isCompleteDefinition()
redundant? Looking at the source for TagDecl::getDefinition() I’m not sure.
-- adrian
>
> + // Make sure the instantiation is actually in a module.
> + if (CTSD->field_begin() != CTSD->field_end())
> + return CTSD->field_begin()->isFromASTFile();
> + }
> + return true;
> +}
> +
> static bool shouldOmitDefinition(codegenoptions::DebugInfoKind DebugKind,
> bool DebugTypeExtRefs, const RecordDecl *RD,
> const LangOptions &LangOpts) {
> - // Does the type exist in an imported clang module?
> - if (DebugTypeExtRefs && RD->isFromASTFile() && RD->getDefinition() &&
> - (RD->isExternallyVisible() || !RD->getName().empty()))
> + if (DebugTypeExtRefs && isDefinedInClangModule(RD))
> return true;
>
> if (DebugKind > codegenoptions::LimitedDebugInfo)
>
> Modified: cfe/trunk/test/Modules/ExtDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.cpp?rev=267464&r1=267463&r2=267464&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ExtDebugInfo.cpp?rev=267464&r1=267463&r2=267464&view=diff>
> ==============================================================================
> --- cfe/trunk/test/Modules/ExtDebugInfo.cpp (original)
> +++ cfe/trunk/test/Modules/ExtDebugInfo.cpp Mon Apr 25 15:52:40 2016
> @@ -2,7 +2,7 @@
> // Test that only forward declarations are emitted for types dfined in
> modules.
>
> // Modules:
> -// RUN: %clang_cc1 -x objective-c++ -std=c++11 -debug-info-kind=limited \
> +// 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 -DMODULES \
> // RUN: -triple %itanium_abi_triple \
> @@ -13,7 +13,7 @@
> // RUN: %clang_cc1 -x c++ -std=c++11 -fmodule-format=obj -emit-pch
> -I%S/Inputs \
> // RUN: -triple %itanium_abi_triple \
> // RUN: -o %t.pch %S/Inputs/DebugCXX.h
> -// RUN: %clang_cc1 -std=c++11 -debug-info-kind=limited \
> +// RUN: %clang_cc1 -std=c++11 -debug-info-kind=standalone \
> // RUN: -dwarf-ext-refs -fmodule-format=obj \
> // RUN: -triple %itanium_abi_triple \
> // RUN: -include-pch %t.pch %s -emit-llvm -o %t-pch.ll %s
> @@ -30,7 +30,9 @@ Struct s;
> DebugCXX::Enum e;
> DebugCXX::Template<long> implicitTemplate;
> DebugCXX::Template<int> explicitTemplate;
> -DebugCXX::FloatInstatiation typedefTemplate;
> +DebugCXX::FloatInstantiation typedefTemplate;
> +DebugCXX::B anchoredTemplate;
> +
> int Struct::static_member = -1;
> enum {
> e3 = -1
> @@ -41,6 +43,11 @@ char _anchor = anon_enum + conflicting_u
> TypedefUnion tdu;
> TypedefEnum tde;
> TypedefStruct tds;
> +TypedefTemplate tdt;
> +Template1<int> explicitTemplate1;
> +
> +template <class T> class FwdDeclTemplate { T t; };
> +TypedefFwdDeclTemplate tdfdt;
>
> InAnonymousNamespace anon;
>
> @@ -48,7 +55,8 @@ void foo() {
> anon.i = GlobalStruct.i = GlobalUnion.i = GlobalEnum;
> }
>
> -// CHECK: ![[STRUCT:[0-9]+]] = !DICompositeType(tag: DW_TAG_structure_type,
> name: "Struct",
> +
> +// CHECK: ![[STRUCT:.*]] = !DICompositeType(tag: DW_TAG_structure_type,
> name: "Struct",
> // CHECK-SAME: scope: ![[NS:[0-9]+]],
> // CHECK-SAME: flags: DIFlagFwdDecl,
> // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
> @@ -61,25 +69,69 @@ void foo() {
> // CHECK-SAME: flags: DIFlagFwdDecl,
> // CHECK-SAME: identifier: "_ZTSN8DebugCXX4EnumE")
>
> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template<int,
> DebugCXX::traits<int> >",
> +// This type is anchored in the module by an explicit template instantiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<long, DebugCXX::traits<long> >",
> // CHECK-SAME: scope: ![[NS]],
> // CHECK-SAME: flags: DIFlagFwdDecl,
> -// CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")
> +// CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE")
>
> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template<float,
> DebugCXX::traits<float> >",
> +// This type is anchored in the module by an explicit template instantiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<int, DebugCXX::traits<int> >",
> // CHECK-SAME: scope: ![[NS]],
> // CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")
> +
> +// This type isn't, however, even with standalone non-module debug info this
> +// type is a forward declaration.
> +// CHECK-NOT: !DICompositeType(tag: DW_TAG_structure_type, name:
> "traits<int>",
> +
> +// This one isn't.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<float, DebugCXX::traits<float> >",
> +// CHECK-SAME: scope: ![[NS]],
> +// CHECK-SAME: templateParams:
> // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
>
> +// This type is anchored in the module by an explicit template instantiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "traits<float>",
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
> +
> +
> +// This type is anchored in the module by an explicit template instantiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>",
> +// CHECK-SAME: scope: ![[NS]],
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
> +
> // CHECK: !DIDerivedType(tag: DW_TAG_member, name: "static_member",
> // CHECK-SAME: scope: ![[STRUCT]]
>
> // CHECK: !DICompositeType(tag: DW_TAG_union_type,
> -// CHECK-SAME: flags: DIFlagFwdDecl, identifier:
> "_ZTS12TypedefUnion")
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTS12TypedefUnion")
> // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type,
> -// CHECK-SAME: flags: DIFlagFwdDecl, identifier:
> "_ZTS11TypedefEnum")
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTS11TypedefEnum")
> // CHECK: !DICompositeType(tag: DW_TAG_structure_type,
> -// CHECK-SAME: flags: DIFlagFwdDecl, identifier:
> "_ZTS13TypedefStruct")
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTS13TypedefStruct")
> +
> +// This one isn't.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template1<void *>",
> +// CHECK-SAME: templateParams:
> +// CHECK-SAME: identifier: "_ZTS9Template1IPvE")
> +
> +// This type is anchored in the module by an explicit template instantiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template1<int>",
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTS9Template1IiE")
> +
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name:
> "FwdDeclTemplate<int>",
> +// CHECK-SAME: templateParams:
> +// CHECK-SAME: identifier: "_ZTS15FwdDeclTemplateIiE")
>
> // CHECK: !DIGlobalVariable(name: "anon_enum", {{.*}}, type:
> ![[ANON_ENUM:[0-9]+]]
> // CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, scope: ![[NS]],
> @@ -94,6 +146,7 @@ void foo() {
> // CHECK: ![[GLOBAL_STRUCT]] = distinct !DICompositeType(tag:
> DW_TAG_structure_type,
> // CHECK-SAME: elements: !{{[0-9]+}})
>
> +
> // CHECK: !DIGlobalVariable(name: "anon",
> // CHECK-SAME: type: ![[GLOBAL_ANON:[0-9]+]]
> // CHECK: ![[GLOBAL_ANON]] = !DICompositeType(tag: DW_TAG_structure_type,
>
> Modified: cfe/trunk/test/Modules/Inputs/DebugCXX.h
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=267464&r1=267463&r2=267464&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugCXX.h?rev=267464&r1=267463&r2=267464&view=diff>
> ==============================================================================
> --- cfe/trunk/test/Modules/Inputs/DebugCXX.h (original)
> +++ cfe/trunk/test/Modules/Inputs/DebugCXX.h Mon Apr 25 15:52:40 2016
> @@ -24,10 +24,11 @@ namespace DebugCXX {
> > class Template {
> T member;
> };
> + // Explicit template instantiation.
> extern template class Template<int>;
>
> extern template struct traits<float>;
> - typedef class Template<float> FloatInstatiation;
> + typedef class Template<float> FloatInstantiation;
>
> inline void fn() {
> Template<long> invisible;
> @@ -48,6 +49,7 @@ namespace DebugCXX {
> template <typename...> class A;
> template <typename T> class A<T> {};
> typedef A<void> B;
> + // Anchored by a function parameter.
> void foo(B) {}
> }
>
> @@ -83,3 +85,13 @@ class Derived : Base {
> Derived *getParent() const override;
> };
> };
> +
> +template <class T>
> +class Template1 {
> + T t;
> +};
> +typedef Template1<void *> TypedefTemplate;
> +extern template class Template1<int>;
> +
> +template <class T> class FwdDeclTemplate;
> +typedef FwdDeclTemplate<int> TypedefFwdDeclTemplate;
>
> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=267464&r1=267463&r2=267464&view=diff
>
> <http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=267464&r1=267463&r2=267464&view=diff>
> ==============================================================================
> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Mon Apr 25 15:52:40 2016
> @@ -47,19 +47,39 @@
> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
> // no mangled name here yet.
>
> +// This type is anchored by a function parameter.
> // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"
> +// CHECK-SAME: templateParams:
> // CHECK-SAME: identifier: "_ZTSN8DebugCXX1AIJvEEE")
>
> // CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "Struct"
> // CHECK-SAME: identifier: "_ZTSN8DebugCXX6StructE")
>
> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template<int,
> DebugCXX::traits<int> >"
> +// This type is anchored by an explicit template instantiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<int, DebugCXX::traits<int> >"
> +// CHECK-SAME: templateParams:
> // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIiNS_6traitsIiEEEE")
>
> -// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "traits<int>"
> +// CHECK-SAME: flags: DIFlagFwdDecl
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIiEE")
> +
> +// CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "traits<float>"
> +// CHECK-SAME: templateParams:
> +// CHECK-SAME: identifier: "_ZTSN8DebugCXX6traitsIfEE")
> +
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<long, DebugCXX::traits<long> >"
> +// CHECK-SAME: templateParams:
> +// CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE")
> +
> +// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstantiation"
> // no mangled name here yet.
>
> -// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template<float,
> DebugCXX::traits<float> >"
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template<float, DebugCXX::traits<float> >"
> +// CHECK-SAME: flags: DIFlagFwdDecl
> // CHECK-SAME: identifier:
> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
>
> // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "FwdVirtual"
> @@ -87,12 +107,28 @@
> // CHECK-SAME: name: "InAnonymousNamespace",
> // CHECK-SAME: elements: !{{[0-9]+}})
>
> -// CHECK: ![[A:[0-9]+]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type,
> name: "A",
> -// CHECK: ![[DERIVED:[0-9]+]] = {{.*}}!DICompositeType(tag:
> DW_TAG_class_type, name: "Derived",
> -// CHECK-SAME: identifier:
> "_ZTS7Derived")
> +// CHECK: ![[DERIVED:.*]] = {{.*}}!DICompositeType(tag: DW_TAG_class_type,
> name: "Derived",
> +// CHECK-SAME: identifier:
> "_ZTS7Derived")
> // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "B", scope:
> ![[DERIVED]],
> -// CHECK-SAME: elements: ![[B_MBRS:.*]], vtableHolder: ![[A]]
> +// CHECK-SAME: elements: ![[B_MBRS:.*]], vtableHolder:
> // CHECK: ![[B_MBRS]] = !{{{.*}}, ![[GET_PARENT:.*]]}
> // CHECK: ![[GET_PARENT]] = !DISubprogram(name: "getParent"
>
> +// CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "TypedefTemplate",
> +// CHECK-SAME: baseType: ![[BASE:.*]])
> +// CHECK: ![[BASE]] = !DICompositeType(tag: DW_TAG_class_type,
> +// CHECK-SAME: name: "Template1<void *>",
> +// CHECK-SAME: flags: DIFlagFwdDecl,
> +// CHECK-SAME: identifier: "_ZTS9Template1IPvE")
> +
> +// Explicit instatiation.
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "Template1<int>",
> +// CHECK-SAME: templateParams:
> +// CHECK-SAME: identifier: "_ZTS9Template1IiE")
> +
> +// CHECK: !DICompositeType(tag: DW_TAG_class_type, name:
> "FwdDeclTemplate<int>",
> +// CHECK-SAME: flags: DIFlagFwdDecl
> +// CHECK-SAME: identifier: "_ZTS15FwdDeclTemplateIiE")
> +
> +
> // CHECK-NEG-NOT: !DICompositeType(tag: DW_TAG_structure_type, name:
> "PureForwardDecl"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
> <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
