> On Oct 2, 2015, at 3:24 PM, David Blaikie <dblai...@gmail.com> wrote: > > > > On Fri, Oct 2, 2015 at 3:21 PM, Adrian Prantl <apra...@apple.com > <mailto:apra...@apple.com>> wrote: > >> On Oct 2, 2015, at 3:01 PM, David Blaikie <dblai...@gmail.com >> <mailto:dblai...@gmail.com>> wrote: >> >> >> >> On Fri, Oct 2, 2015 at 3:00 PM, Adrian Prantl <apra...@apple.com >> <mailto:apra...@apple.com>> wrote: >> >>> On Oct 2, 2015, at 2:58 PM, David Blaikie <dblai...@gmail.com >>> <mailto:dblai...@gmail.com>> wrote: >>> >>> >>> >>> On Fri, Oct 2, 2015 at 2:40 PM, Adrian Prantl <apra...@apple.com >>> <mailto:apra...@apple.com>> wrote: >>> >>>> On Oct 2, 2015, at 2:18 PM, David Blaikie <dblai...@gmail.com >>>> <mailto:dblai...@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 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. -- adrian > > > -- adrian > >> >> >> -- adrian >> >>> >>> >>> -- adrian >>> >>>> On Fri, Oct 2, 2015 at 10:36 AM, Adrian Prantl via cfe-commits >>>> <cfe-commits@lists.llvm.org <mailto:cfe-commits@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 >>>> <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 >>>> >>>> <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 >>>> >>>> <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 >>>> >>>> <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@lists.llvm.org <mailto:cfe-commits@lists.llvm.org> >>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >>>> <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
