On Fri, Sep 5, 2014 at 1:24 PM, Ben Langmuir <[email protected]> wrote:
> Author: benlangmuir > Date: Fri Sep 5 15:24:27 2014 > New Revision: 217275 > > URL: http://llvm.org/viewvc/llvm-project?rev=217275&view=rev > Log: > Move the initialization of VAListTagName after InitializeSema() > > This innocuous statement to get the identifier info for __va_list_tag > was causing an assertion failure: > NextIsPrevious() && "decl became non-canonical unexpectedly" > if the __va_list_tag identifier was found in a PCH in some > circumstances, because it was looked up before the ASTReader had a Sema > object to use to find existing decls to merge with. > > We could possibly move getting the identifier info even later, or make > it lazy if we wanted to, but this seemed like the minimal change. > > Now why a PCH would have this identifier in the first place is a bit > mysterious. This seems to be related to the global module index in some > way, because when the test case is built without the global module index > it will not emit an identifier for __va_list_tag into the PCH, but with > the global module index it does. > > Added: > cfe/trunk/test/Modules/Inputs/va_list/ > cfe/trunk/test/Modules/Inputs/va_list/module.modulemap > cfe/trunk/test/Modules/Inputs/va_list/va_list_a.h > cfe/trunk/test/Modules/Inputs/va_list/va_list_b.h > cfe/trunk/test/Modules/va_list.m > Modified: > cfe/trunk/lib/Sema/Sema.cpp > > Modified: cfe/trunk/lib/Sema/Sema.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/Sema.cpp?rev=217275&r1=217274&r2=217275&view=diff > > ============================================================================== > --- cfe/trunk/lib/Sema/Sema.cpp (original) > +++ cfe/trunk/lib/Sema/Sema.cpp Fri Sep 5 15:24:27 2014 > @@ -69,8 +69,6 @@ PrintingPolicy Sema::getPrintingPolicy(c > void Sema::ActOnTranslationUnitScope(Scope *S) { > TUScope = S; > PushDeclContext(S, Context.getTranslationUnitDecl()); > - > - VAListTagName = PP.getIdentifierInfo("__va_list_tag"); > } > > Sema::Sema(Preprocessor &pp, ASTContext &ctxt, ASTConsumer &consumer, > @@ -155,6 +153,10 @@ void Sema::Initialize() { > = dyn_cast_or_null<ExternalSemaSource>(Context.getExternalSource())) > ExternalSema->InitializeSema(*this); > > + // This needs to happen after ExternalSemaSource::InitializeSema(this) > or we > + // will not be able to merge any duplicate __va_list_tag decls > correctly. > + VAListTagName = PP.getIdentifierInfo("__va_list_tag"); > + > // Initialize predefined 128-bit integer types, if needed. > if (Context.getTargetInfo().hasInt128Type()) { > // If either of the 128-bit integer types are unavailable to name > lookup, > > Added: cfe/trunk/test/Modules/Inputs/va_list/module.modulemap > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/va_list/module.modulemap?rev=217275&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/va_list/module.modulemap (added) > +++ cfe/trunk/test/Modules/Inputs/va_list/module.modulemap Fri Sep 5 > 15:24:27 2014 > @@ -0,0 +1,5 @@ > +module stdarg [system] { > + header "stdarg.h" // note: supplied by the compiler > I've simplified this test in 217290 so it doesn't depend on the presence of system (or compiler) headers. It looks like it didn't need it (I guess objc provides va_list by default?). Let me know if there's more to it than that, but I did verify that the test still (with my changes) fails without your patch. > +} > +module va_list_a { header "va_list_a.h" } > +module va_list_b { header "va_list_b.h" } > > Added: cfe/trunk/test/Modules/Inputs/va_list/va_list_a.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/va_list/va_list_a.h?rev=217275&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/va_list/va_list_a.h (added) > +++ cfe/trunk/test/Modules/Inputs/va_list/va_list_a.h Fri Sep 5 15:24:27 > 2014 > @@ -0,0 +1,2 @@ > +@import stdarg; > +int vprintf(const char * __restrict, va_list); > > Added: cfe/trunk/test/Modules/Inputs/va_list/va_list_b.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/va_list/va_list_b.h?rev=217275&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/va_list/va_list_b.h (added) > +++ cfe/trunk/test/Modules/Inputs/va_list/va_list_b.h Fri Sep 5 15:24:27 > 2014 > @@ -0,0 +1,2 @@ > +@import va_list_a; > +void NSLogv(id, va_list); > > Added: cfe/trunk/test/Modules/va_list.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/va_list.m?rev=217275&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/va_list.m (added) > +++ cfe/trunk/test/Modules/va_list.m Fri Sep 5 15:24:27 2014 > @@ -0,0 +1,27 @@ > +// RUN: rm -rf %t > +// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -fmodules > -fmodules-cache-path=%t \ > +// RUN: -fmodules-ignore-macro=PREFIX -DPREFIX -I %S/Inputs/va_list \ > +// RUN: -x objective-c-header %s -o %t.pch -emit-pch > + > +// Include the pch, as a sanity check. > +// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -fmodules > -fmodules-cache-path=%t \ > +// RUN: -fmodules-ignore-macro=PREFIX -I %S/Inputs/va_list > -include-pch %t.pch \ > +// RUN: -x objective-c %s -fsyntax-only > + > +// Repeat the previous emit-pch, but not we will have a global module > index. > +// For some reason, this results in an identifier for __va_list_tag being > +// emitted into the pch. > +// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -fmodules > -fmodules-cache-path=%t \ > +// RUN: -fmodules-ignore-macro=PREFIX -DPREFIX -I %S/Inputs/va_list \ > +// RUN: -x objective-c-header %s -o %t.pch -emit-pch > + > +// Include the pch, which now has __va_list_tag in it, which needs to be > merged. > +// RUN: %clang_cc1 -triple x86_64-apple-macosx10 -fmodules > -fmodules-cache-path=%t \ > +// RUN: -fmodules-ignore-macro=PREFIX -I %S/Inputs/va_list > -include-pch %t.pch \ > +// RUN: -x objective-c %s -fsyntax-only > + > +// rdar://18039719 > + > +#ifdef PREFIX > +@import va_list_b; > +#endif > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
