r217275 - Move the initialization of VAListTagName after InitializeSema()
Ben Langmuir
blangmuir at apple.com
Fri Sep 5 16:57:43 PDT 2014
> On Sep 5, 2014, at 4:48 PM, David Blaikie <dblaikie at gmail.com> wrote:
>
>
>
>
> On Fri, Sep 5, 2014 at 1:24 PM, Ben Langmuir <blangmuir at apple.com <mailto:blangmuir at apple.com>> 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 <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 <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 <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.
Weird - I thought I tried that and it didn’t work. Thanks for the simplification!
Ben
>
> +}
> +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 <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 @@
> + at 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 <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 @@
> + at 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 <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
> + at import va_list_b;
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu <mailto:cfe-commits at cs.uiuc.edu>
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits <http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140905/f7b988bf/attachment.html>
More information about the cfe-commits
mailing list