> On Apr 26, 2016, at 4:11 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
>
> On Tue, Apr 26, 2016 at 3:10 PM, Adrian Prantl via cfe-commits
> <cfe-commits@lists.llvm.org <mailto:cfe-commits@lists.llvm.org>> wrote:
>
>> On Apr 25, 2016, at 5:34 PM, Richard Smith <rich...@metafoo.co.uk
>> <mailto: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?
>
> Something just like your testcase for class template specializations should
> work:
>
> // DebugCXX.h
> template <class T> class FwdDeclTemplateMember { class Member; };
> typedef FwdDeclTemplateMember<int>::Member TypedefFwdDeclTemplateMember;
>
> // ExtDebugInfo.cpp
> template <class T> class FwdDeclTemplateMember<T>::Member { T t; };
> TypedefFwdDeclTemplateMember tdfdtm;
>> + 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?
>
> Yes. The only other possibility here is that the class is currently being
> defined (we're between the open and close braces), but as far as I know you
> shouldn't ever see such a class from here, and you should certainly never see
> one where the partial definition is imported from an AST file. You could
> assert isCompleteDefinition() if you're worried that we might give you a bad
> AST.
FYI: The -gmodules bot on green dragon recently found a situation where this
assertion fired. You might be interested in the testcase I added in r279485.
-- adrian
>
> -- 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 <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
