<div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Wed, 24 Oct 2018 at 14:20, Adrian Prantl via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org">cfe-dev@lists.llvm.org</a>> wrote:<br></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;line-break:after-white-space"><div><blockquote type="cite"><div>On Oct 24, 2018, at 1:36 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br class="m_-7840188483273717214Apple-interchange-newline"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><div class="gmail_quote"><div dir="ltr">On Wed, 24 Oct 2018 at 12:51, Adrian Prantl via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div>On Oct 23, 2018, at 6:17 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br class="m_-7840188483273717214m_5286080070532041288Apple-interchange-newline"><div><div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px;text-decoration:none"><div class="gmail_quote"><div dir="ltr">On Tue, 23 Oct 2018 at 17:29, Adrian Prantl via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><div style="word-wrap:break-word;line-break:after-white-space"><div><blockquote type="cite"><div>On Oct 23, 2018, at 5:10 PM, Richard Smith <<a href="mailto:richard@metafoo.co.uk" target="_blank">richard@metafoo.co.uk</a>> wrote:</div><br class="m_-7840188483273717214m_5286080070532041288m_8430990931295308993Apple-interchange-newline"><div><div dir="ltr"><div class="gmail_quote"><div dir="ltr">On Mon, 15 Oct 2018 at 10:13, Adrian Prantl via cfe-dev <<a href="mailto:cfe-dev@lists.llvm.org" target="_blank">cfe-dev@lists.llvm.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">I'm trying to fix a bug in the -gmodules debug info that appears when debugging Clang with local submodule visibility enabled. In my reduced testcase, I have a module A, defining a template "AlignedCharArray<unsigned, unsigned>", and two templates "SmallVector" and "Optional" that use it. In module B, I'm instantiating an Optional that instantiates an AlignedCharArray<4, 16>. In module C, I'm instantiating a SmallVector that also instantiates an AlignedCharArray<4, 16>.<br>The -gmodules debug info format saves space by forward-declaring types that are defined in an already-imported module and specify which module they came from. To point to the module of the definition CGDebugInfo::getParentModuleOrNull(const Decl *D) uses Decl::getOwningModule(). This mechanism works as expected, but only if the TemplateDecl is not inside a namespace.<br><br>When compiling the attached testcase with -U WITH_NAMESPACE, the owning module for the ClassTemplateSpecializationDecl (of AlignedCharArray<4, 16>) in module B is C. (B imports C, so the C specialization is instantiated first).<br>When compiling the attached testcase with -D WITH_NAMESPACE, the owning module for the ClassTemplateSpecializationDecl (of AlignedCharArray<4, 16>) in module B is A. (A is where the TemplateDecl that is being specialized comes from).<br><br>I'm sure that this discrepancy must be because a namespace can be spread out over many modules.</blockquote><div><br></div><div>Sort of. In practice, what's happening is this:</div><div><br></div><div>When we create a declaration, we inherit its owning module from its lexical parent by default. And we ensure that the lexical parent of the TU declaration corresponds to the module we're currently "in". When a declaration is injected into a lexical declaration context from a different module, such as happens when adding a template specialization to an imported template, that default behavior will pick up the current module at global scope, but will pick up the owning module of the enclosing namespace declaration at namespace scope, which will be the owning module of whichever declaration of the template we picked. Now, this usually doesn't matter, because a template specialization doesn't really *have* a meaningful notion of "owning module". For visibility purposes, the specialization should be visible wherever the corresponding template is visible, and for linkage purposes, it should be considered owned by the same module as the template pattern. So in some sense the bug is in the global-scope case: we should always treat the specialization as being owned by module A.</div><div><br></div><div>I think there's a chance that you're asking the wrong question -- perhaps you should be trying to determine whether the declaration in question is imported from a module *file*, not what its semantic owning module is. (As an example, suppose we're tracking module ownership but not using PCM files. If we textually enter a modular header of some other module, I think you want to *not* treat declarations in that header as being owned by a module for the purpose of -gmodules, because there is no reason to think there's separate debug information for that header.) You can figure that out by calling Decl::isFromASTFile(), and if you want to know *which* AST file the declaration comes from (because you know that some AST files have debug information and some don't), you can use ASTReader::getOwningModuleFile to find out.</div></div></div></div></blockquote><div><br></div><div>Thanks for the pointer! That interface sounds promising; I will try to expose it as a</div><div><br></div><div>   virtual ASTSourceDescriptor getOwningModuleFile(Decl *);</div><div><br></div><div>in ExternalASTSource and see if that gets me further.</div></div></div></blockquote><div><br></div><div>A more general name might make sense at the ExternalASTSource level (eg, lldb might want to specify that its declarations come from a certain .dwo file), but that generally sounds reasonable to me.</div><div> </div></div></div></div></blockquote><div><br></div><div>I experimented with this quite a bit today, and using getOwningModuleFile() solves 50% of my problem. Not bad :-) </div><div>Using it, I can now correctly deduce the Module that contains the previously imported definition of the ClassTemplateSpecialization, and emit debug info as a  forward decl.</div><div><br></div><div>But while emitting debug info for the PCM that contains the definition of ClassTemplateSpecializationDecl itself, getOwningModuleFile() errors out because Decl::isFromASTFile() is naturally false and the function only works for imported types; so the template specialization ends up nested in the module that contains the ClassTemplate, not the one that contains the specialization.</div><div><br></div><div>Is there a way to determine the module ClassTemplateSpecializationDecl is in while compiling that module? The ClassTemplateSpecializationDecl's DeclContext and LexicalDeclContext both are the NamespaceDecl in the module containing the ClassTemplate. I suppose that while compiling a PCM, I could force every Decl that !isFromASTFile() to be inside that PCM's module, but it would be nice to have something more elegant.</div></div></div></blockquote><div><br></div><div>I think the same approach should work, but maybe I'm not understanding something about the problem? If getOwningModuleFile returns null, then the declaration is not imported from a module file, so you should do with it whatever you're doing with declarations that aren't from an imported module file. (Eg, if you're building a module file with modular debug information, then emit debug information for that declaration.)</div></div></div></div></blockquote></div><br><div>The problem is that in the debug info I'm nesting the type declarations inside module declarations that replicate the (sub)module structure, but I can't determine (sub)module a declaration is in correctly in this case.</div><div><br></div><div>That said, I just had an offline discussion with Bruno: The reason why I'm nesting debug info in submodules is so we can distinguish ambiguously-named types in two submodules of the same PCM. But with the latest LSV work, it is actually illegal to do this even in (Objective-)C, so I might be able to define the whole problem away by omitting the module information for typed defined (not imported) inside PCMs. I'll explore this some more.</div></div></blockquote><div><br></div><div>I think there are really two different things going on here: the module file would determine whether you emit debug information as part of this compilation, and the owning module for linkage would determine which module to nest the debug information within in the DWARF. For a template instantiation, the owning module for linkage *should* be the owning module of the template, because that's what determines whether two template specializations are specializations of the same template.</div><div><br></div><div>For standard C++ modules, we likely will need the module wrapper in DWARF, because it's valid to define two (non-exported) templates with the same name in two different modules, and it's even possible to trigger instantiations of both templates with the same arguments from a single translation unit. Eg:<br></div><div><br></div><div>export module M1;</div><div>template<typename T> struct A {};</div><div>export template<typename T> auto f1() { return A<T>(); }</div><div><br></div><div><div>export module M2;</div><div>template<typename T> struct A {};</div><div>export template<typename T> auto f2() { return A<T>(); }</div></div><div><br></div><div>// elsewhere</div><div>import M1;</div><div>import M2;</div><div>auto a = f1<int>(); // a is of type A@M1<int></div><div>auto b = f2<int>(); // b is of type A@M2<int></div><div><br></div><div>It'd make sense to fix Clang's AST representation so the owning module of a template specialization is set to be the owning module of the template. (Currently we instead mark template specializations as unconditionally-visible, which is not quite right.)</div></div></div>