r267464 - Module Debugging: Fix the condition for determining whether a template
Adrian Prantl via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 26 16:49:24 PDT 2016
> On Apr 26, 2016, at 4:11 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Tue, Apr 26, 2016 at 3:10 PM, Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>> wrote:
>
>> On Apr 25, 2016, at 5:34 PM, Richard Smith <richard at metafoo.co.uk <mailto:richard at metafoo.co.uk>> wrote:
>>
>> On Mon, Apr 25, 2016 at 1:52 PM, Adrian Prantl via cfe-commits <cfe-commits at lists.llvm.org <mailto:cfe-commits at 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;
Thanks, that does the trick! Fixed in r267630.
>> + 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.
I added the assertion in r267633.
-- 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 at lists.llvm.org <mailto:cfe-commits at 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 at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160426/31ae5a53/attachment-0001.html>
More information about the cfe-commits
mailing list