Great, thanks! On Thu, Mar 24, 2016, 5:26 PM Adrian Prantl <apra...@apple.com> wrote:
> I added a testcase that demonstrates the problem with this commit in > r264366. > > -- adrian > > On Feb 23, 2016, at 11:07 AM, Eric Christopher <echri...@gmail.com> > wrote: > > > > Thanks Adrian! > > > > On Tue, Feb 23, 2016 at 9:19 AM Adrian Prantl <apra...@apple.com> wrote: > > --> r261657. > > > > Author: adrian <adrian@91177308-0d34-0410-b5e6-96231b3b80d8> > > Date: Tue Feb 23 17:13:47 2016 +0000 > > > > Remove an unnecessary workaround introduced in r259975. (NFC) > > > > Now that LLVM r259973 allows replacing a temporary type with another > > temporary we can rely on the original implementation. > > > > It is possible for enums to be created as part of > > their own declcontext. In this case a FwdDecl will be created > > twice. This doesn't cause a problem because both FwdDecls are > > entered into the ReplaceMap: finalize() will replace the first > > FwdDecl with the second and then replace the second with > > complete type. > > > > Thanks to echristo for pointing this out. > > > > git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@261657 > > > > -- adrian > > > > > On Feb 22, 2016, at 5:26 PM, Eric Christopher <echri...@gmail.com> > wrote: > > > > > > Hi Adrian, > > > > > > On Sat, Feb 20, 2016 at 1:18 PM Adrian Prantl <apra...@apple.com> > wrote: > > > This had me puzzled for a second, but then I figured out what happened > :-) > > > The “crash” I quoted in the commit message really was an assertion > failure, to be precise, the very assertion I relaxed in LLVM r259973 in > order to be able to defer the building of the DeclContext implemented by > this commit. Even with the assertion removed, I still believe that > implementing it this way preferable, as it avoids creating the enum a > second time. > > > > > > > On Feb 19, 2016, at 7:05 PM, Eric Christopher <echri...@gmail.com> > wrote: > > > > > > > > Hi Adrian, > > > > > > > > I'm taking a look at this and can't duplicate using the testcase you > gave without your patch(es) applied. It's also causing asserts in other > code as you can have the forward decl left around and the CU isn't a valid > context. > > > > > > Do you happen to have an example handy? I don’t quite understand the > problem from the description — shouldn’t the temporary fwddecl be RAUWed > unconditionally after the DeclContext is created? > > > > > > > > > I'd think so. It appears to get through to code generation unreplaced > though and... > > > > > > If you’re blocked on this we can definitely revert the change in > CGDebugInfo (but not the testcase), I just would like to understand what’s > going on. > > > > > > > > > I am a bit. I don't have a reduction at the moment small enough to > debug what's wrong with this and the current patch doesn't appear necessary > so I'm unable to debug the original problem at this point to try to help > more. > > > > > > -eric > > > > > > -- adrian > > > > > > > Can you take a look/revert until you've got a different testcase? > There's not enough information in the commit to construct one up for you. > > > > > > > > Thanks! > > > > > > > > -eric > > > > > > > > On Fri, Feb 5, 2016 at 6:03 PM Adrian Prantl via cfe-commits < > cfe-commits@lists.llvm.org> wrote: > > > > Author: adrian > > > > Date: Fri Feb 5 19:59:09 2016 > > > > New Revision: 259975 > > > > > > > > URL: http://llvm.org/viewvc/llvm-project?rev=259975&view=rev > > > > Log: > > > > Fix a crash when emitting dbeug info for forward-declared scoped > enums. > > > > It is possible for enums to be created as part of their own > > > > declcontext. We need to cache a placeholder to avoid the type being > > > > created twice before hitting the cache. > > > > > > > > <rdar://problem/24493203> > > > > > > > > Added: > > > > cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp > > > > Modified: > > > > cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > > > > > > > > Modified: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp > > > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGDebugInfo.cpp?rev=259975&r1=259974&r2=259975&view=diff > > > > > ============================================================================== > > > > --- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp (original) > > > > +++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp Fri Feb 5 19:59:09 2016 > > > > @@ -2051,13 +2051,25 @@ llvm::DIType *CGDebugInfo::CreateEnumTyp > > > > // If this is just a forward declaration, construct an > appropriately > > > > // marked node and just return it. > > > > if (isImportedFromModule || !ED->getDefinition()) { > > > > - llvm::DIScope *EDContext = getDeclContextDescriptor(ED); > > > > llvm::DIFile *DefUnit = getOrCreateFile(ED->getLocation()); > > > > + > > > > + // It is possible for enums to be created as part of their own > > > > + // declcontext. We need to cache a placeholder to avoid the > type being > > > > + // created twice before hitting the cache. > > > > + llvm::DIScope *EDContext = > DBuilder.createReplaceableCompositeType( > > > > + llvm::dwarf::DW_TAG_enumeration_type, "", TheCU, DefUnit, 0); > > > > + > > > > unsigned Line = getLineNumber(ED->getLocation()); > > > > StringRef EDName = ED->getName(); > > > > llvm::DIType *RetTy = DBuilder.createReplaceableCompositeType( > > > > llvm::dwarf::DW_TAG_enumeration_type, EDName, EDContext, > DefUnit, Line, > > > > 0, Size, Align, llvm::DINode::FlagFwdDecl, FullName); > > > > + > > > > + // Cache the enum type so it is available when building the > declcontext > > > > + // and replace the declcontect with the real thing. > > > > + TypeCache[Ty].reset(RetTy); > > > > + EDContext->replaceAllUsesWith(getDeclContextDescriptor(ED)); > > > > + > > > > ReplaceMap.emplace_back( > > > > std::piecewise_construct, std::make_tuple(Ty), > > > > std::make_tuple(static_cast<llvm::Metadata *>(RetTy))); > > > > > > > > Added: cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp > > > > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp?rev=259975&view=auto > > > > > ============================================================================== > > > > --- cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp (added) > > > > +++ cfe/trunk/test/CodeGenCXX/debug-info-scoped-class.cpp Fri Feb 5 > 19:59:09 2016 > > > > @@ -0,0 +1,15 @@ > > > > +// RUN: %clang_cc1 -emit-llvm -debug-info-kind=standalone > -std=c++11 \ > > > > +// RUN: -triple thumbv7-apple-ios %s -o - | FileCheck %s > > > > + > > > > +// This forward-declared scoped enum will be created while building > its own > > > > +// declcontext. Make sure it is only emitted once. > > > > + > > > > +struct A { > > > > + enum class Return; > > > > + Return f1(); > > > > +}; > > > > +A::Return* f2() {} > > > > + > > > > +// CHECK: !DICompositeType(tag: DW_TAG_enumeration_type, name: > "Return", > > > > +// CHECK-SAME: flags: DIFlagFwdDecl, > > > > +// CHECK-NOT: tag: DW_TAG_enumeration_type, name: > "Return" > > > > > > > > > > > > _______________________________________________ > > > > cfe-commits mailing list > > > > cfe-commits@lists.llvm.org > > > > 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
