<html><head><meta http-equiv="Content-Type" content="text/html charset=us-ascii"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; -webkit-line-break: after-white-space;" class="">Yep, that should be it!<div class=""><br class=""><div><blockquote type="cite" class=""><div class="">On Aug 13, 2015, at 4:45 PM, Sean Silva <<a href="mailto:chisophugis@gmail.com" class="">chisophugis@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class="">This was the last thing blocking <a href="http://reviews.llvm.org/D10423" rel="noreferrer" style="font-size:13px" target="_blank" class="">http://reviews.llvm.org/D10423</a> on your side, right?<div class=""><br class=""></div><div class="">-- Sean Silva</div></div><div class="gmail_extra"><br class=""><div class="gmail_quote">On Thu, Aug 13, 2015 at 10:13 AM, Ben Langmuir via cfe-commits <span dir="ltr" class=""><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank" class="">cfe-commits@lists.llvm.org</a>></span> wrote:<br class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: benlangmuir<br class="">
Date: Thu Aug 13 12:13:33 2015<br class="">
New Revision: 244912<br class="">
<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=244912&view=rev" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project?rev=244912&view=rev</a><br class="">
Log:<br class="">
[Modules] Add Darwin-specific compatibility module map parsing hacks<br class="">
<br class="">
This preserves backwards compatibility for two hacks in the Darwin<br class="">
system module map files:<br class="">
<br class="">
1. The use of 'requires excluded' to make headers non-modular, which<br class="">
should really be mapped to 'textual' now that we have this feature.<br class="">
<br class="">
2. Silently removes a bogus cplusplus requirement from IOKit.avc.<br class="">
<br class="">
Once we start diagnosing missing requirements and headers on<br class="">
auto-imports these would have broken compatibility with existing Darwin<br class="">
SDKs.<br class="">
<br class="">
Added:<br class="">
cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h<br class="">
cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/<br class="">
cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h<br class="">
cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m<br class="">
Modified:<br class="">
cfe/trunk/include/clang/Basic/Module.h<br class="">
cfe/trunk/lib/Basic/Module.cpp<br class="">
cfe/trunk/lib/Lex/ModuleMap.cpp<br class="">
cfe/trunk/test/Modules/Inputs/System/usr/include/module.map<br class="">
<br class="">
Modified: cfe/trunk/include/clang/Basic/Module.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=244912&r1=244911&r2=244912&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=244912&r1=244911&r2=244912&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/include/clang/Basic/Module.h (original)<br class="">
+++ cfe/trunk/include/clang/Basic/Module.h Thu Aug 13 12:13:33 2015<br class="">
@@ -356,6 +356,12 @@ public:<br class="">
/// its top-level module.<br class="">
std::string getFullModuleName() const;<br class="">
<br class="">
+ /// \brief Whether the full name of this module is equal to joining<br class="">
+ /// \p nameParts with "."s.<br class="">
+ ///<br class="">
+ /// This is more efficient than getFullModuleName().<br class="">
+ bool fullModuleNameIs(ArrayRef<StringRef> nameParts) const;<br class="">
+<br class="">
/// \brief Retrieve the top-level module for this (sub)module, which may<br class="">
/// be this module.<br class="">
Module *getTopLevelModule() {<br class="">
<br class="">
Modified: cfe/trunk/lib/Basic/Module.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=244912&r1=244911&r2=244912&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=244912&r1=244911&r2=244912&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/lib/Basic/Module.cpp (original)<br class="">
+++ cfe/trunk/lib/Basic/Module.cpp Thu Aug 13 12:13:33 2015<br class="">
@@ -139,6 +139,15 @@ std::string Module::getFullModuleName()<br class="">
return Result;<br class="">
}<br class="">
<br class="">
+bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {<br class="">
+ for (const Module *M = this; M; M = M->Parent) {<br class="">
+ if (nameParts.empty() || M->Name != nameParts.back())<br class="">
+ return false;<br class="">
+ nameParts = nameParts.drop_back();<br class="">
+ }<br class="">
+ return nameParts.empty();<br class="">
+}<br class="">
+<br class="">
Module::DirectoryName Module::getUmbrellaDir() const {<br class="">
if (Header U = getUmbrellaHeader())<br class="">
return {"", U.Entry->getDir()};<br class="">
<br class="">
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=244912&r1=244911&r2=244912&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=244912&r1=244911&r2=244912&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)<br class="">
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Thu Aug 13 12:13:33 2015<br class="">
@@ -1015,7 +1015,17 @@ namespace clang {<br class="">
<br class="">
/// \brief The active module.<br class="">
Module *ActiveModule;<br class="">
-<br class="">
+<br class="">
+ /// \brief Whether a module uses the 'requires excluded' hack to mark its<br class="">
+ /// contents as 'textual'.<br class="">
+ ///<br class="">
+ /// On older Darwin SDK versions, 'requires excluded' is used to mark the<br class="">
+ /// contents of the Darwin.C.excluded (assert.h) and Tcl.Private modules as<br class="">
+ /// non-modular headers. For backwards compatibility, we continue to<br class="">
+ /// support this idiom for just these modules, and map the headers to<br class="">
+ /// 'textual' to match the original intent.<br class="">
+ llvm::SmallPtrSet<Module *, 2> UsesRequiresExcludedHack;<br class="">
+<br class="">
/// \brief Consume the current token and return its location.<br class="">
SourceLocation consumeToken();<br class="">
<br class="">
@@ -1570,6 +1580,38 @@ void ModuleMapParser::parseExternModuleD<br class="">
: File->getDir(), ExternLoc);<br class="">
}<br class="">
<br class="">
+/// Whether to add the requirement \p Feature to the module \p M.<br class="">
+///<br class="">
+/// This preserves backwards compatibility for two hacks in the Darwin system<br class="">
+/// module map files:<br class="">
+///<br class="">
+/// 1. The use of 'requires excluded' to make headers non-modular, which<br class="">
+/// should really be mapped to 'textual' now that we have this feature. We<br class="">
+/// drop the 'excluded' requirement, and set \p IsRequiresExcludedHack to<br class="">
+/// true. Later, this bit will be used to map all the headers inside this<br class="">
+/// module to 'textual'.<br class="">
+///<br class="">
+/// This affects Darwin.C.excluded (for assert.h) and Tcl.Private.<br class="">
+///<br class="">
+/// 2. Removes a bogus cplusplus requirement from IOKit.avc. This requirement<br class="">
+/// was never correct and causes issues now that we check it, so drop it.<br class="">
+static bool shouldAddRequirement(Module *M, StringRef Feature,<br class="">
+ bool &IsRequiresExcludedHack) {<br class="">
+ static const StringRef DarwinCExcluded[] = {"Darwin", "C", "excluded"};<br class="">
+ static const StringRef TclPrivate[] = {"Tcl", "Private"};<br class="">
+ static const StringRef IOKitAVC[] = {"IOKit", "avc"};<br class="">
+<br class="">
+ if (Feature == "excluded" && (M->fullModuleNameIs(DarwinCExcluded) ||<br class="">
+ M->fullModuleNameIs(TclPrivate))) {<br class="">
+ IsRequiresExcludedHack = true;<br class="">
+ return false;<br class="">
+ } else if (Feature == "cplusplus" && M->fullModuleNameIs(IOKitAVC)) {<br class="">
+ return false;<br class="">
+ }<br class="">
+<br class="">
+ return true;<br class="">
+}<br class="">
+<br class="">
/// \brief Parse a requires declaration.<br class="">
///<br class="">
/// requires-declaration:<br class="">
@@ -1605,9 +1647,18 @@ void ModuleMapParser::parseRequiresDecl(<br class="">
std::string Feature = Tok.getString();<br class="">
consumeToken();<br class="">
<br class="">
- // Add this feature.<br class="">
- ActiveModule->addRequirement(Feature, RequiredState,<br class="">
- Map.LangOpts, *Map.Target);<br class="">
+ bool IsRequiresExcludedHack = false;<br class="">
+ bool ShouldAddRequirement =<br class="">
+ shouldAddRequirement(ActiveModule, Feature, IsRequiresExcludedHack);<br class="">
+<br class="">
+ if (IsRequiresExcludedHack)<br class="">
+ UsesRequiresExcludedHack.insert(ActiveModule);<br class="">
+<br class="">
+ if (ShouldAddRequirement) {<br class="">
+ // Add this feature.<br class="">
+ ActiveModule->addRequirement(Feature, RequiredState, Map.LangOpts,<br class="">
+ *Map.Target);<br class="">
+ }<br class="">
<br class="">
if (!<a href="http://tok.is" class="">Tok.is</a>(MMToken::Comma))<br class="">
break;<br class="">
@@ -1657,9 +1708,16 @@ void ModuleMapParser::parseHeaderDecl(MM<br class="">
consumeToken();<br class="">
}<br class="">
}<br class="">
+<br class="">
if (LeadingToken == MMToken::TextualKeyword)<br class="">
Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);<br class="">
<br class="">
+ if (UsesRequiresExcludedHack.count(ActiveModule)) {<br class="">
+ // Mark this header 'textual' (see doc comment for<br class="">
+ // Module::UsesRequiresExcludedHack).<br class="">
+ Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);<br class="">
+ }<br class="">
+<br class="">
if (LeadingToken != MMToken::HeaderKeyword) {<br class="">
if (!<a href="http://tok.is" class="">Tok.is</a>(MMToken::HeaderKeyword)) {<br class="">
Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header)<br class="">
@@ -1838,14 +1896,40 @@ void ModuleMapParser::parseUmbrellaDirDe<br class="">
HadError = true;<br class="">
return;<br class="">
}<br class="">
-<br class="">
+<br class="">
+ if (UsesRequiresExcludedHack.count(ActiveModule)) {<br class="">
+ // Mark this header 'textual' (see doc comment for<br class="">
+ // ModuleMapParser::UsesRequiresExcludedHack). Although iterating over the<br class="">
+ // directory is relatively expensive, in practice this only applies to the<br class="">
+ // uncommonly used Tcl module on Darwin platforms.<br class="">
+ std::error_code EC;<br class="">
+ SmallVector<Module::Header, 6> Headers;<br class="">
+ for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(), EC), E;<br class="">
+ I != E && !EC; I.increment(EC)) {<br class="">
+ if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path())) {<br class="">
+<br class="">
+ Module::Header Header = {I->path(), FE};<br class="">
+ Headers.push_back(std::move(Header));<br class="">
+ }<br class="">
+ }<br class="">
+<br class="">
+ // Sort header paths so that the pcm doesn't depend on iteration order.<br class="">
+ llvm::array_pod_sort(Headers.begin(), Headers.end(),<br class="">
+ [](const Module::Header *A, const Module::Header *B) {<br class="">
+ return A->NameAsWritten.compare(B->NameAsWritten);<br class="">
+ });<br class="">
+ for (auto &Header : Headers)<br class="">
+ Map.addHeader(ActiveModule, std::move(Header), ModuleMap::TextualHeader);<br class="">
+ return;<br class="">
+ }<br class="">
+<br class="">
if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {<br class="">
Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)<br class="">
<< OwningModule->getFullModuleName();<br class="">
HadError = true;<br class="">
return;<br class="">
- }<br class="">
-<br class="">
+ }<br class="">
+<br class="">
// Record this umbrella directory.<br class="">
Map.setUmbrellaDir(ActiveModule, Dir, DirName);<br class="">
}<br class="">
<br class="">
Added: cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h?rev=244912&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h?rev=244912&view=auto</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h (added)<br class="">
+++ cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h Thu Aug 13 12:13:33 2015<br class="">
@@ -0,0 +1,2 @@<br class="">
+// assert.h<br class="">
+#define DARWIN_C_EXCLUDED 1<br class="">
<br class="">
Modified: cfe/trunk/test/Modules/Inputs/System/usr/include/module.map<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/module.map?rev=244912&r1=244911&r2=244912&view=diff" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/module.map?rev=244912&r1=244911&r2=244912&view=diff</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/Modules/Inputs/System/usr/include/module.map (original)<br class="">
+++ cfe/trunk/test/Modules/Inputs/System/usr/include/module.map Thu Aug 13 12:13:33 2015<br class="">
@@ -30,3 +30,25 @@ module uses_other_constants {<br class="">
header "uses_other_constants.h"<br class="">
export *<br class="">
}<br class="">
+<br class="">
+module Darwin {<br class="">
+ module C {<br class="">
+ module excluded {<br class="">
+ requires excluded<br class="">
+ header "assert.h"<br class="">
+ }<br class="">
+ }<br class="">
+}<br class="">
+<br class="">
+module Tcl {<br class="">
+ module Private {<br class="">
+ requires excluded<br class="">
+ umbrella ""<br class="">
+ }<br class="">
+}<br class="">
+<br class="">
+module IOKit {<br class="">
+ module avc {<br class="">
+ requires cplusplus<br class="">
+ }<br class="">
+}<br class="">
<br class="">
Added: cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h?rev=244912&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h?rev=244912&view=auto</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h (added)<br class="">
+++ cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h Thu Aug 13 12:13:33 2015<br class="">
@@ -0,0 +1,2 @@<br class="">
+// tcl-private/header.h<br class="">
+#define TCL_PRIVATE 1<br class="">
<br class="">
Added: cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m<br class="">
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m?rev=244912&view=auto" rel="noreferrer" target="_blank" class="">http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m?rev=244912&view=auto</a><br class="">
==============================================================================<br class="">
--- cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m (added)<br class="">
+++ cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m Thu Aug 13 12:13:33 2015<br class="">
@@ -0,0 +1,22 @@<br class="">
+// RUN: rm -rf %t<br class="">
+// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t -fimplicit-module-maps -isystem %S/Inputs/System/usr/include -triple x86_64-apple-darwin10 %s -verify -fsyntax-only<br class="">
+// expected-no-diagnostics<br class="">
+<br class="">
+@import Darwin.C.excluded; // no error, header is implicitly 'textual'<br class="">
+@import Tcl.Private; // no error, header is implicitly 'textual'<br class="">
+@import IOKit.avc; // no error, cplusplus requirement removed<br class="">
+<br class="">
+#if defined(DARWIN_C_EXCLUDED)<br class="">
+#error assert.h should be textual<br class="">
+#elif defined(TCL_PRIVATE)<br class="">
+#error tcl-private/header.h should be textual<br class="">
+#endif<br class="">
+<br class="">
+#import <assert.h><br class="">
+#import <tcl-private/header.h><br class="">
+<br class="">
+#if !defined(DARWIN_C_EXCLUDED)<br class="">
+#error assert.h missing<br class="">
+#elif !defined(TCL_PRIVATE)<br class="">
+#error tcl-private/header.h missing<br class="">
+#endif<br class="">
<br class="">
<br class="">
_______________________________________________<br class="">
cfe-commits mailing list<br class="">
<a href="mailto:cfe-commits@lists.llvm.org" class="">cfe-commits@lists.llvm.org</a><br class="">
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank" class="">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br class="">
</blockquote></div><br class=""></div>
</div></blockquote></div><br class=""></div></body></html>