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