r267464 - Module Debugging: Fix the condition for determining whether a template
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Tue Apr 26 16:11:54 PDT 2016
On Tue, Apr 26, 2016 at 3:10 PM, Adrian Prantl via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
>
> On Apr 25, 2016, at 5:34 PM, Richard Smith <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> 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
>> 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
>> 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
>>
>> ==============================================================================
>> --- 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.
-- 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
>>
>> ==============================================================================
>> --- 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
>>
>> ==============================================================================
>> --- 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
>>
>> ==============================================================================
>> --- 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
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> 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/a4d53111/attachment-0001.html>
More information about the cfe-commits
mailing list