r249157 - Module debugging: Don't emit forward declarations in module scopes.

David Blaikie via cfe-commits cfe-commits at lists.llvm.org
Mon Oct 5 11:37:29 PDT 2015


On Mon, Oct 5, 2015 at 8:55 AM, Adrian Prantl <aprantl at apple.com> wrote:

>
> On Oct 2, 2015, at 3:24 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
> On Fri, Oct 2, 2015 at 3:21 PM, Adrian Prantl <aprantl at apple.com> wrote:
>
>>
>> On Oct 2, 2015, at 3:01 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>
>>
>>
>> On Fri, Oct 2, 2015 at 3:00 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>
>>>
>>> On Oct 2, 2015, at 2:58 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>
>>>
>>>
>>> On Fri, Oct 2, 2015 at 2:40 PM, Adrian Prantl <aprantl at apple.com> wrote:
>>>
>>>>
>>>> On Oct 2, 2015, at 2:18 PM, David Blaikie <dblaikie at gmail.com> wrote:
>>>>
>>>> This seems a little curious, so you'll have code like this:
>>>>
>>>>   decl foo
>>>>   module
>>>>     def bar
>>>>        member foo pointer (references the declaration of foo outside
>>>> the module, in the CU?)
>>>>
>>>>
>>>> Right.
>>>>
>>>> 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?
>>>>
>>>>
>>>> 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.
>>>>
>>>
>>> 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)
>>>
>>> Shouldn't dsymutil be treating modules more like CUs as "another top
>>> level scope”?
>>>
>>>
>>> The CU is not part of the DeclContext (for dsymutil’s interpretation of
>>> a DeclContext). Otherwise cross-CU type uniquing would never work.
>>>
>>
>> perhaps the module shouldn't be either, then?
>>
>>
>> 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).
>> 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.
>>
>
> 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?
>
>
> 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.
> 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
>

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)

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?

(& 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)


> 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.
>

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)).


>
> -- adrian
>
>
>
>>
>> -- adrian
>>
>>
>>
>>>
>>> -- adrian
>>>
>>>
>>>
>>>>
>>>> -- adrian
>>>>
>>>> On Fri, Oct 2, 2015 at 10:36 AM, Adrian Prantl via cfe-commits <
>>>> cfe-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: adrian
>>>>> Date: Fri Oct  2 12:36:14 2015
>>>>> New Revision: 249157
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=249157&view=rev
>>>>> Log:
>>>>> Module debugging: Don't emit forward declarations in module scopes.
>>>>> A forward declaration inside a module header does not belong to the
>>>>> module.
>>>>>
>>>>> Modified:
>>>>>     cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>>>>     cfe/trunk/test/Modules/Inputs/DebugObjC.h
>>>>>     cfe/trunk/test/Modules/ModuleDebugInfo.m
>>>>>
>>>>> Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=249157&r1=249156&r2=249157&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original)
>>>>> +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Oct  2 12:36:14 2015
>>>>> @@ -2172,6 +2172,9 @@ ObjCInterfaceDecl *CGDebugInfo::getObjCI
>>>>>  }
>>>>>
>>>>>  llvm::DIModule *CGDebugInfo::getParentModuleOrNull(const Decl *D) {
>>>>> +  // A forward declaration inside a module header does not belong to
>>>>> the module.
>>>>> +  if (isa<RecordDecl>(D) && !cast<RecordDecl>(D)->getDefinition())
>>>>> +    return nullptr;
>>>>>    if (DebugTypeExtRefs && D->isFromASTFile()) {
>>>>>      // Record a reference to an imported clang module or precompiled
>>>>> header.
>>>>>      auto *Reader = CGM.getContext().getExternalSource();
>>>>>
>>>>> Modified: cfe/trunk/test/Modules/Inputs/DebugObjC.h
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/DebugObjC.h?rev=249157&r1=249156&r2=249157&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/test/Modules/Inputs/DebugObjC.h (original)
>>>>> +++ cfe/trunk/test/Modules/Inputs/DebugObjC.h Fri Oct  2 12:36:14 2015
>>>>> @@ -5,6 +5,7 @@
>>>>>  }
>>>>>  + classMethod;
>>>>>  - instanceMethodWithInt:(int)i;
>>>>> +- (struct OpaqueData*) getSomethingOpaque;
>>>>>  @property int property;
>>>>>  @end
>>>>>
>>>>>
>>>>> Modified: cfe/trunk/test/Modules/ModuleDebugInfo.m
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ModuleDebugInfo.m?rev=249157&r1=249156&r2=249157&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/test/Modules/ModuleDebugInfo.m (original)
>>>>> +++ cfe/trunk/test/Modules/ModuleDebugInfo.m Fri Oct  2 12:36:14 2015
>>>>> @@ -41,3 +41,6 @@
>>>>>  // MODULE-CHECK: !DICompositeType(tag: DW_TAG_structure_type,
>>>>>  // MODULE-CHECK-SAME:             name: "ObjCClass",
>>>>>  // MODULE-CHECK-SAME:             scope: ![[MODULE]],
>>>>> +
>>>>> +// The forward declaration should not be in the module scope.
>>>>> +// MODULE-CHECK: !DICompositeType(tag: DW_TAG_structure_type, name:
>>>>> "OpaqueData", file
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> 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/20151005/51532609/attachment-0001.html>


More information about the cfe-commits mailing list