<div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Oct 5, 2015 at 11:37 AM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Mon, Oct 5, 2015 at 8:55 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><span><blockquote type="cite"><div>On Oct 2, 2015, at 3:24 PM, 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 Fri, Oct 2, 2015 at 3:21 PM, Adrian Prantl<span> </span><span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.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 style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Oct 2, 2015, at 3:01 PM, 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 Fri, Oct 2, 2015 at 3:00 PM, Adrian Prantl<span> </span><span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.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 style="word-wrap:break-word"><span><br><div><blockquote type="cite"><div>On Oct 2, 2015, at 2:58 PM, 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 Fri, Oct 2, 2015 at 2:40 PM, Adrian Prantl<span> </span><span dir="ltr"><<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.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 style="word-wrap:break-word"><br><div><span><blockquote type="cite"><div>On Oct 2, 2015, at 2:18 PM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br><div><div dir="ltr">This seems a little curious, so you'll have code like this:<br><br> <span> </span>decl foo<br> <span> </span>module<br>   <span> </span>def bar<br>       member foo pointer (references the declaration of foo outside the module, in the CU?)<br><br></div></div></blockquote><div><br></div></span><div>Right.</div><span><br><blockquote type="cite"><div dir="ltr">Why is that preferable to DWARF that looks more like the AST where the declaration of foo appears in the first module that references it, rather than, in a curiously circular situation, in the CU that references the module?<br></div></blockquote><div><br></div></span><div>The problem I had with this was that a forward declaration would end up in a DeclContext that is not the DeclContext of the definition. Looking at this through the dsymutil goggles, the different DeclContexts effectively prevent the forward declaration from being uniqued with the type's definition.</div></div></div></blockquote><div><br></div><div>Putting it in the CU doesn't seem to make the decl context match either, does it? In that case the decl context may still be "some other compile unit" (as with most declarations)<br><br>Shouldn't dsymutil be treating modules more like CUs as "another top level scope”?</div></div></div></blockquote><br></div></span><div>The CU is not part of the DeclContext (for dsymutil’s interpretation of a DeclContext). Otherwise cross-CU type uniquing would never work.</div></div></blockquote><div><br></div><div>perhaps the module shouldn't be either, then?</div></div></div></blockquote><div><br></div></span>The module has to be, for correctness. (None of this applies to C++. Oversimplified: because of the ODR we can omit the parent DW_TAG_module from all CXXRecordDecls).</div><div>For C and ObjC it is perfectly legal to have two modules with conflicting definitions for a type, so we need the DW_TAG_module as part of the DeclContext to differentiate them if we want to be able to resolve forward declarations to the correct definition.</div></div></blockquote><div><br></div><div>But you have to do the same thing for the things in the CU as well, right? So you can't ignore the CU in the same way you can't ignore the module, yes?</div></div></div></blockquote><div><br></div></span><div>You’ve got a point there. In a non-ODR language, you still wouldn’t be allowed to resolve a forward declaration to a specific type.</div><div>The one thing that I don’t like about emitting “real" forward declarations into the DW_TAG_module is that they are indistinguishable from the forward declarations that are emitted for external type references, for which there is an actual definition in the module itself. This is more of an aesthetic problem </div></div></div></blockquote><div><br></div></div></div><div>Yeah, I agree it's a bit weird & not sure what the nicest solution is, but I think this is a bit more consistent with the way the modules actually work (that the type appears in the first module it's used from, even if it's just a declaration, then other modules do actually reference the type from the prior one)<br><br>Remind me why we need these other types in the non-defining module anyway? Since it only contains type declarations, when does it need to contain types that aren't defined in the module? It just happens to be that its a type declaration that comes from the module, I guess? But the type will never be referenced from anything inside the non-defining DW_TAG_module?<br></div></div></div></div></blockquote><div><br></div><div>(to clarify further - if nothing in the non-defining DW_TAG_module references the type declaration, and the type definition won't be found in the module, yeah - I think aesthetically, it makes sense to put the type declaration in the CU - it's just the way the code to handle that falls out that feels weird, I think)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr"><div class="gmail_extra"><div class="gmail_quote"><div><br>(& I know this is revisiting decisions we've made - but again, I wonder if this would all fall out more naturally if modularized types were treated more traditionally, but with an extra attribute in their declaration if they happen to be defined in a module we know about at the moment - so the code would look more like -flimit-debug-info just "if this definition is from a module, just emit a declaration" (& the "if we're building a declaration for a type defined in a module and we're targetting lldb, add the extra attribute) - it does seem a little (but not massively so) more complicated to be having to deal with these different context chains)</div><span class=""><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><div>than a technical one, but it does make dsymutils job of resolving type references in a single pass a little harder since dsymutil can no longer assume that all forward declarations in a non-defining TAG_module can be resolved.</div></div></div></blockquote><div><br></div></span><div>Wouldn't dsymutil have a pretty generic algorithm for resolving declarations to definitions that would have to cope with missing definitions for declarations in CUs? So I would imagine this case would just "fall out" of that resolution code without need for any special cases. (so long as you visit modules in reverse order of dependencies - so you visit the dependencies before the modules they depend on (or the other way around, if that works best)).</div><div><div class="h5"><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><div><div><div><br></div><div>-- adrian</div><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 style="word-wrap:break-word"><span><font color="#888888"><div><br></div></font></span><div><span><font color="#888888">-- adrian</font></span><div><div><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 style="word-wrap:break-word"><span><font color="#888888"><div><br></div><div>-- adrian</div></font></span><div><div><div><br></div><div><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 style="word-wrap:break-word"><div><span><font color="#888888"><div><br></div><div>-- adrian</div></font></span><div><div><div><br></div><blockquote type="cite"><div><div class="gmail_extra"><div class="gmail_quote">On Fri, Oct 2, 2015 at 10:36 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: Fri Oct  2 12:36:14 2015<br>New Revision: 249157<br><br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project?rev=249157&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=249157&view=rev</a><br>Log:<br>Module debugging: Don't emit forward declarations in module scopes.<br>A forward declaration inside a module header does not belong to the module.<br><br>Modified:<br>   <span> </span>cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>   <span> </span>cfe/trunk/test/Modules/Inputs/DebugObjC.h<br>   <span> </span>cfe/trunk/test/Modules/ModuleDebugInfo.m<br><br>Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=249157&r1=249156&r2=249157&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=249157&r1=249156&r2=249157&view=diff</a><br>==============================================================================<br>--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)<br>+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Oct  2 12:36:14 2015<br>@@ -2172,6 +2172,9 @@ ObjCInterfaceDecl *CGDebugInfo::getObjCI<br> }<br><br> llvm::DIModule *CGDebugInfo::getParentModuleOrNull(const Decl *D) {<br>+  // A forward declaration inside a module header does not belong to the module.<br>+  if (isa<RecordDecl>(D) && !cast<RecordDecl>(D)->getDefinition())<br>+    return nullptr;<br>   if (DebugTypeExtRefs && D->isFromASTFile()) {<br>     // Record a reference to an imported clang module or precompiled header.<br>     auto *Reader = CGM.getContext().getExternalSource();<br><br>Modified: cfe/trunk/test/Modules/Inputs/DebugObjC.h<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugObjC.h?rev=249157&r1=249156&r2=249157&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugObjC.h?rev=249157&r1=249156&r2=249157&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Modules/Inputs/DebugObjC.h (original)<br>+++ cfe/trunk/test/Modules/Inputs/DebugObjC.h Fri Oct  2 12:36:14 2015<br>@@ -5,6 +5,7 @@<br> }<br> + classMethod;<br> - instanceMethodWithInt:(int)i;<br>+- (struct OpaqueData*) getSomethingOpaque;<br> @property int property;<br> @end<br><br><br>Modified: cfe/trunk/test/Modules/ModuleDebugInfo.m<br>URL:<span> </span><a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.m?rev=249157&r1=249156&r2=249157&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.m?rev=249157&r1=249156&r2=249157&view=diff</a><br>==============================================================================<br>--- cfe/trunk/test/Modules/ModuleDebugInfo.m (original)<br>+++ cfe/trunk/test/Modules/ModuleDebugInfo.m Fri Oct  2 12:36:14 2015<br>@@ -41,3 +41,6 @@<br> // MODULE-CHECK: !DICompositeType(tag: DW_TAG_structure_type,<br> // MODULE-CHECK-SAME:             name: "ObjCClass",<br> // MODULE-CHECK-SAME:             scope: ![[MODULE]],<br>+<br>+// The forward declaration should not be in the module scope.<br>+// MODULE-CHECK: !DICompositeType(tag: DW_TAG_structure_type, name: "OpaqueData", file<br><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></div></div></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div></div></div></div></blockquote></div></div></blockquote></div></div></div><br></div></blockquote></div></div></div><br></div></div>
</blockquote></div><br></div></div>