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