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