On Tue, Sep 6, 2016 at 11:16 AM, Manman Ren via cfe-commits < cfe-commits@lists.llvm.org> wrote:
> Author: mren > Date: Tue Sep 6 13:16:54 2016 > New Revision: 280728 > > URL: http://llvm.org/viewvc/llvm-project?rev=280728&view=rev > Log: > Modules: Fix an assertion in DeclContext::buildLookup. > > When calling getMostRecentDecl, we can pull in more definitions from > a module. We call getPrimaryContext afterwards to make sure that > we buildLookup on a primary context. > > rdar://27926200 > > Added: > cfe/trunk/test/Modules/Inputs/lookup-assert/ > cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h > cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h > cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h > cfe/trunk/test/Modules/Inputs/lookup-assert/module.map > cfe/trunk/test/Modules/lookup-assert.m > Modified: > cfe/trunk/lib/AST/DeclBase.cpp > > Modified: cfe/trunk/lib/AST/DeclBase.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ > DeclBase.cpp?rev=280728&r1=280727&r2=280728&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/AST/DeclBase.cpp (original) > +++ cfe/trunk/lib/AST/DeclBase.cpp Tue Sep 6 13:16:54 2016 > @@ -1411,10 +1411,6 @@ DeclContext::lookup(DeclarationName Name > assert(DeclKind != Decl::LinkageSpec && > "Should not perform lookups into linkage specs!"); > > - const DeclContext *PrimaryContext = getPrimaryContext(); > - if (PrimaryContext != this) > - return PrimaryContext->lookup(Name); > - > // If we have an external source, ensure that any later redeclarations > of this > // context have been loaded, since they may add names to the result of > this > // lookup (or add external visible storage). > @@ -1422,6 +1418,12 @@ DeclContext::lookup(DeclarationName Name > if (Source) > (void)cast<Decl>(this)->getMostRecentDecl(); > > + // getMostRecentDecl can change the result of getPrimaryContext. Call > + // getPrimaryContext afterwards. > + const DeclContext *PrimaryContext = getPrimaryContext(); > + if (PrimaryContext != this) > + return PrimaryContext->lookup(Name); > This seems like a bug in getPrimaryContext() -- if there is a not-yet-loaded definition of the DeclContext, getPrimaryContext() should return that instead of returning a non-defining declaration. Why is ObjCInterfaceDecl::hasDefinition (indirectly called by getPrimaryContext) not loading the definition in this case? > + > if (hasExternalVisibleStorage()) { > assert(Source && "external visible storage but no external source?"); > > > Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/lookup-assert/Base.h?rev=280728&view=auto > ============================================================ > ================== > --- cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h (added) > +++ cfe/trunk/test/Modules/Inputs/lookup-assert/Base.h Tue Sep 6 > 13:16:54 2016 > @@ -0,0 +1,4 @@ > +@interface BaseInterface > +- (void) test; > +@end > + > > Added: cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/lookup-assert/Derive.h?rev=280728&view=auto > ============================================================ > ================== > --- cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h (added) > +++ cfe/trunk/test/Modules/Inputs/lookup-assert/Derive.h Tue Sep 6 > 13:16:54 2016 > @@ -0,0 +1,3 @@ > +#include "Base.h" > +@interface DerivedInterface : BaseInterface > +@end > > Added: cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/lookup-assert/H3.h?rev=280728&view=auto > ============================================================ > ================== > --- cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h (added) > +++ cfe/trunk/test/Modules/Inputs/lookup-assert/H3.h Tue Sep 6 13:16:54 > 2016 > @@ -0,0 +1 @@ > +#include "Base.h" > > Added: cfe/trunk/test/Modules/Inputs/lookup-assert/module.map > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/Inputs/lookup-assert/module.map?rev=280728&view=auto > ============================================================ > ================== > --- cfe/trunk/test/Modules/Inputs/lookup-assert/module.map (added) > +++ cfe/trunk/test/Modules/Inputs/lookup-assert/module.map Tue Sep 6 > 13:16:54 2016 > @@ -0,0 +1,4 @@ > +module X { > + header "H3.h" > + export * > +} > > Added: cfe/trunk/test/Modules/lookup-assert.m > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/ > Modules/lookup-assert.m?rev=280728&view=auto > ============================================================ > ================== > --- cfe/trunk/test/Modules/lookup-assert.m (added) > +++ cfe/trunk/test/Modules/lookup-assert.m Tue Sep 6 13:16:54 2016 > @@ -0,0 +1,10 @@ > +// RUN: rm -rf %t > +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules > -fimplicit-module-maps -I %S/Inputs/lookup-assert %s -verify > +// expected-no-diagnostics > + > +#include "Derive.h" > +#import <H3.h> > +@implementation DerivedInterface > +- (void)test { > +} > +@end > > > _______________________________________________ > 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