r247303 - Debug Info: Remove an unnecessary debug type visitor.

[David Blaikie via cfe-commits]

On Thu, Sep 10, 2015 at 10:18 AM, David Blaikie <dblaikie at gmail.com> wrote:

>
>
> On Thu, Sep 10, 2015 at 10:13 AM, Adrian Prantl via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: adrian
>> Date: Thu Sep 10 12:13:31 2015
>> New Revision: 247303
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=247303&view=rev
>> Log:
>> Debug Info: Remove an unnecessary debug type visitor.
>> Thanks to dblaikie for spotting this.
>>
>> Modified:
>>     cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
>>     cfe/trunk/test/Modules/ModuleDebugInfo.cpp
>>
>> Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303&r1=247302&r2=247303&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)
>> +++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Sep 10
>> 12:13:31 2015
>> @@ -70,13 +70,6 @@ class PCHContainerGenerator : public AST
>>        return true;
>>      }
>>
>> -    bool VisitValueDecl(ValueDecl *D) {
>> -      QualType QualTy = D->getType();
>> -      if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
>> -        DI.getOrCreateStandaloneType(QualTy, D->getLocation());
>> -      return true;
>> -    }
>> -
>>      bool VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {
>>        QualType QualTy(D->getTypeForDecl(), 0);
>>        if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))
>>
>> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303&r1=247302&r2=247303&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)
>> +++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Thu Sep 10 12:13:31 2015
>> @@ -7,10 +7,12 @@
>>  // RUN: rm -rf %t
>>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x objective-c++
>> -std=c++11 -g -fmodules -fmodule-format=obj -fimplicit-module-maps
>> -DMODULES -fmodules-cache-path=%t %s -I %S/Inputs -I %t -emit-llvm -o %t.ll
>> -mllvm -debug-only=pchcontainer &>%t-mod.ll
>>  // RUN: cat %t-mod.ll | FileCheck %s
>> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
>>
>>  // PCH:
>>  // RUN: %clang_cc1 -triple %itanium_abi_triple -x c++ -std=c++11
>> -emit-pch -fmodule-format=obj -I %S/Inputs -o %t.pch %S/Inputs/DebugCXX.h
>> -mllvm -debug-only=pchcontainer &>%t-pch.ll
>>  // RUN: cat %t-pch.ll | FileCheck %s
>> +// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s
>>
>>  #ifdef MODULES
>>  @import DebugCXX;
>> @@ -30,12 +32,11 @@
>>  // CHECK: !DICompositeType(tag: DW_TAG_class_type,
>>  // CHECK-SAME:             name: "Template<float,
>> DebugCXX::traits<float> >"
>>  // CHECK-SAME:             identifier:
>> "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")
>> -// CHECK: !DICompositeType(tag: DW_TAG_class_type,
>> -// CHECK-SAME:             name: "Template<long, DebugCXX::traits<long>
>> >"
>> -// CHECK-SAME:             identifier:
>> "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE")
>>  // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"
>>  // CHECK-SAME:             identifier: "_ZTSN8DebugCXX1AIJvEEE")
>>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"
>>  // no mangled name here yet.
>>  // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",
>>  // no mangled name here yet.
>> +
>> +// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE"
>>
>
> Rather than using a separate check - maybe do the same sort of thing we do
> for DWARF testing, CHECK-NOT between each DICompositeType, to ensure we
> only get the types we intended? (including a CHECK-NOT at the end to ensure
> there aren't any trailing ones)
>
> But also: How does the current implementation avoid emitting this type? I
> thought it visited all the type decls, even those not immediately in the
> module? (you mentioned that was something that you were planning to address
> in a future patch) Is that not the case? Is this now sufficient to only
> emit the decls immediately in this module?
>

(Oh, I guess what might be missing is types referenced from /types/ in this
module - types referenced from variable decls only are addressed by this
patch, but you still don't want to include a full definition of std::vector
just because a type derives from it, has a member of it, etc)


>
>
>>
>>
>> _______________________________________________
>> 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/20150910/957a57c0/attachment.html>