<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Sep 10, 2015 at 10:25 AM, Adrian Prantl <span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><br><div><div><div class="h5"><blockquote type="cite"><div>On Sep 10, 2015, at 10:19 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><br><br style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">On Thu, Sep 10, 2015 at 10:18 AM, David Blaikie<span> </span><span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div>On Thu, Sep 10, 2015 at 10:13 AM, Adrian Prantl via cfe-commits<span> </span><span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span><span> </span>wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex">Author: adrian<br>Date: Thu Sep 10 12:13:31 2015<br>New Revision: 247303<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=247303&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=247303&view=rev</a><br>Log:<br>Debug Info: Remove an unnecessary debug type visitor.<br>Thanks to dblaikie for spotting this.<br><br>Modified:<br>   <span> </span>cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp<br>   <span> </span>cfe/trunk/test/Modules/ModuleDebugInfo.cpp<br><br>Modified: cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303&r1=247302&r2=247303&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp?rev=247303&r1=247302&r2=247303&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp (original)<br>+++ cfe/trunk/lib/CodeGen/ObjectFilePCHContainerOperations.cpp Thu Sep 10 12:13:31 2015<br>@@ -70,13 +70,6 @@ class PCHContainerGenerator : public AST<br>       return true;<br>     }<br><br>-    bool VisitValueDecl(ValueDecl *D) {<br>-      QualType QualTy = D->getType();<br>-      if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))<br>-        DI.getOrCreateStandaloneType(QualTy, D->getLocation());<br>-      return true;<br>-    }<br>-<br>     bool VisitObjCInterfaceDecl(ObjCInterfaceDecl *D) {<br>       QualType QualTy(D->getTypeForDecl(), 0);<br>       if (!QualTy.isNull() && CanRepresent(QualTy.getTypePtr()))<br><br>Modified: cfe/trunk/test/Modules/ModuleDebugInfo.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303&r1=247302&r2=247303&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.cpp?rev=247303&r1=247302&r2=247303&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Modules/ModuleDebugInfo.cpp (original)<br>+++ cfe/trunk/test/Modules/ModuleDebugInfo.cpp Thu Sep 10 12:13:31 2015<br>@@ -7,10 +7,12 @@<br> // RUN: rm -rf %t<br> // 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<br> // RUN: cat %t-mod.ll | FileCheck %s<br>+// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s<br><br> // PCH:<br> // 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<br> // RUN: cat %t-pch.ll | FileCheck %s<br>+// RUN: cat %t-mod.ll | FileCheck --check-prefix=CHECK-NEG %s<br><br> #ifdef MODULES<br> @import DebugCXX;<br>@@ -30,12 +32,11 @@<br> // CHECK: !DICompositeType(tag: DW_TAG_class_type,<br> // CHECK-SAME:             name: "Template<float, DebugCXX::traits<float> >"<br> // CHECK-SAME:             identifier: "_ZTSN8DebugCXX8TemplateIfNS_6traitsIfEEEE")<br>-// CHECK: !DICompositeType(tag: DW_TAG_class_type,<br>-// CHECK-SAME:             name: "Template<long, DebugCXX::traits<long> >"<br>-// CHECK-SAME:             identifier: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE")<br> // CHECK: !DICompositeType(tag: DW_TAG_class_type, name: "A<void>"<br> // CHECK-SAME:             identifier: "_ZTSN8DebugCXX1AIJvEEE")<br> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "FloatInstatiation"<br> // no mangled name here yet.<br> // CHECK: !DIDerivedType(tag: DW_TAG_typedef, name: "B",<br> // no mangled name here yet.<br>+<br>+// CHECK-NEG-NOT: "_ZTSN8DebugCXX8TemplateIlNS_6traitsIlEEEE"<br></blockquote></div></div><div><br>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)<br><br>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?<br></div></div></div></div></blockquote></div></div></blockquote><div><br></div></div></div>It transitively emits all types that are showing up in a type declaration in the module. This type is only instantiated inside a variable declaration.</div><div><span class=""><br><blockquote type="cite"><div class="gmail_quote"></div></blockquote><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div>(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)<br></div></div></div></blockquote><div><br></div></span><div>Unless there is a typedef for that instantiation of std::vector in a module I don’t see a way of avoiding this currently.</div></div></div></blockquote><div><br></div><div>Not sure I follow - you mean you're not sure how to detect that the type (std::vector<int>, say, in this example) is defined within the module or just referenced by it? Or even if you could detect that there's some other difficult/limitation that makes it unavoidable?</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><span class="HOEnZb"><font color="#888888"><div><br></div>-- adrian</font></span><span class=""><br><blockquote type="cite"><div><div class="gmail_quote" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><div> </div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div> </div><span><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-color:rgb(204,204,204);border-left-style:solid;padding-left:1ex"><br><br>_______________________________________________<br>cfe-commits mailing list<br><a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br><a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a></blockquote></span></div></div></div></blockquote></div></div></blockquote></span></div><br></div></blockquote></div><br></div></div>