[cfe-dev] local submodule visibility, namespaces and Decl::getOwningModule()

[Richard Smith via cfe-dev]

On Wed, 24 Oct 2018 at 14:20, Adrian Prantl via cfe-dev <
cfe-dev at lists.llvm.org> wrote:

> On Oct 24, 2018, at 1:36 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> On Wed, 24 Oct 2018 at 12:51, Adrian Prantl via cfe-dev <
> cfe-dev at lists.llvm.org> wrote:
>
>> On Oct 23, 2018, at 6:17 PM, Richard Smith <richard at metafoo.co.uk> wrote:
>>
>> On Tue, 23 Oct 2018 at 17:29, Adrian Prantl via cfe-dev <
>> cfe-dev at lists.llvm.org> wrote:
>>
>>> On Oct 23, 2018, at 5:10 PM, Richard Smith <richard at metafoo.co.uk>
>>> wrote:
>>>
>>> On Mon, 15 Oct 2018 at 10:13, Adrian Prantl via cfe-dev <
>>> cfe-dev at lists.llvm.org> wrote:
>>>
>>>> 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>.
>>>> 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.
>>>>
>>>> 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).
>>>> 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).
>>>>
>>>> I'm sure that this discrepancy must be because a namespace can be
>>>> spread out over many modules.
>>>
>>>
>>> Sort of. In practice, what's happening is this:
>>>
>>> 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.
>>>
>>> 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.
>>>
>>>
>>> Thanks for the pointer! That interface sounds promising; I will try to
>>> expose it as a
>>>
>>>    virtual ASTSourceDescriptor getOwningModuleFile(Decl *);
>>>
>>> in ExternalASTSource and see if that gets me further.
>>>
>>
>> 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.
>>
>>
>>
>> I experimented with this quite a bit today, and using
>> getOwningModuleFile() solves 50% of my problem. Not bad :-)
>> 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.
>>
>> 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.
>>
>> 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.
>>
>
> 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.)
>
>
> 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.
>
> 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.
>

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.

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:

export module M1;
template<typename T> struct A {};
export template<typename T> auto f1() { return A<T>(); }

export module M2;
template<typename T> struct A {};
export template<typename T> auto f2() { return A<T>(); }

// elsewhere
import M1;
import M2;
auto a = f1<int>(); // a is of type A at M1<int>
auto b = f2<int>(); // b is of type A at M2<int>

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.)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-dev/attachments/20181024/3b54ad30/attachment.html>