[Modules] Add Darwin-specific compatibility module map parsing hacks

This preserves backwards compatibility for two hacks in the Darwin
system module map files:

1. The use of 'requires excluded' to make headers non-modular, which
should really be mapped to 'textual' now that we have this feature.

2. Silently removes a bogus cplusplus requirement from IOKit.avc.

Once we start diagnosing missing requirements and headers on
auto-imports these would have broken compatibility with existing Darwin
SDKs.

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@244912 91177308-0d34-0410-b5e6-96231b3b80d8

diff --git a/include/clang/Basic/Module.h b/include/clang/Basic/Module.h
index 1bc89257d38468dcc7a215118e08e5a5698de704..e3060456dfb22dfbd1a2275d6d2e6b0be104f316 100644 (file)
--- a/include/clang/Basic/Module.h
+++ b/include/clang/Basic/Module.h
@@ -356,6 +356,12 @@ public:
   /// its top-level module.
   std::string getFullModuleName() const;
 
+  /// \brief Whether the full name of this module is equal to joining
+  /// \p nameParts with "."s.
+  ///
+  /// This is more efficient than getFullModuleName().
+  bool fullModuleNameIs(ArrayRef<StringRef> nameParts) const;
+
   /// \brief Retrieve the top-level module for this (sub)module, which may
   /// be this module.
   Module *getTopLevelModule() {

diff --git a/lib/Basic/Module.cpp b/lib/Basic/Module.cpp
index 4314b41eb340f81162b484d7b665077ba0c0001f..4188cefc8498f613fa1f7d0c09a5584f275c3b99 100644 (file)
--- a/lib/Basic/Module.cpp
+++ b/lib/Basic/Module.cpp
@@ -139,6 +139,15 @@ std::string Module::getFullModuleName() const {
   return Result;
 }
 
+bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {
+  for (const Module *M = this; M; M = M->Parent) {
+    if (nameParts.empty() || M->Name != nameParts.back())
+      return false;
+    nameParts = nameParts.drop_back();
+  }
+  return nameParts.empty();
+}
+
 Module::DirectoryName Module::getUmbrellaDir() const {
   if (Header U = getUmbrellaHeader())
     return {"", U.Entry->getDir()};

diff --git a/lib/Lex/ModuleMap.cpp b/lib/Lex/ModuleMap.cpp
index 995e1370ede945017ede48ee5899cea1cbd9b647..a1c2d23fa38a88aafc6ba691a45c84ba9a14c7e9 100644 (file)
--- a/lib/Lex/ModuleMap.cpp
+++ b/lib/Lex/ModuleMap.cpp
@@ -1015,7 +1015,17 @@ namespace clang {
     
     /// \brief The active module.
     Module *ActiveModule;
-    
+
+    /// \brief Whether a module uses the 'requires excluded' hack to mark its
+    /// contents as 'textual'.
+    ///
+    /// On older Darwin SDK versions, 'requires excluded' is used to mark the
+    /// contents of the Darwin.C.excluded (assert.h) and Tcl.Private modules as
+    /// non-modular headers.  For backwards compatibility, we continue to
+    /// support this idiom for just these modules, and map the headers to
+    /// 'textual' to match the original intent.
+    llvm::SmallPtrSet<Module *, 2> UsesRequiresExcludedHack;
+
     /// \brief Consume the current token and return its location.
     SourceLocation consumeToken();
     
@@ -1570,6 +1580,38 @@ void ModuleMapParser::parseExternModuleDecl() {
             : File->getDir(), ExternLoc);
 }
 
+/// Whether to add the requirement \p Feature to the module \p M.
+///
+/// This preserves backwards compatibility for two hacks in the Darwin system
+/// module map files:
+///
+/// 1. The use of 'requires excluded' to make headers non-modular, which
+///    should really be mapped to 'textual' now that we have this feature.  We
+///    drop the 'excluded' requirement, and set \p IsRequiresExcludedHack to
+///    true.  Later, this bit will be used to map all the headers inside this
+///    module to 'textual'.
+///
+///    This affects Darwin.C.excluded (for assert.h) and Tcl.Private.
+///
+/// 2. Removes a bogus cplusplus requirement from IOKit.avc.  This requirement
+///    was never correct and causes issues now that we check it, so drop it.
+static bool shouldAddRequirement(Module *M, StringRef Feature,
+                                 bool &IsRequiresExcludedHack) {
+  static const StringRef DarwinCExcluded[] = {"Darwin", "C", "excluded"};
+  static const StringRef TclPrivate[] = {"Tcl", "Private"};
+  static const StringRef IOKitAVC[] = {"IOKit", "avc"};
+
+  if (Feature == "excluded" && (M->fullModuleNameIs(DarwinCExcluded) ||
+                                M->fullModuleNameIs(TclPrivate))) {
+    IsRequiresExcludedHack = true;
+    return false;
+  } else if (Feature == "cplusplus" && M->fullModuleNameIs(IOKitAVC)) {
+    return false;
+  }
+
+  return true;
+}
+
 /// \brief Parse a requires declaration.
 ///
 ///   requires-declaration:
@@ -1605,9 +1647,18 @@ void ModuleMapParser::parseRequiresDecl() {
     std::string Feature = Tok.getString();
     consumeToken();
 
-    // Add this feature.
-    ActiveModule->addRequirement(Feature, RequiredState,
-                                 Map.LangOpts, *Map.Target);
+    bool IsRequiresExcludedHack = false;
+    bool ShouldAddRequirement =
+        shouldAddRequirement(ActiveModule, Feature, IsRequiresExcludedHack);
+
+    if (IsRequiresExcludedHack)
+      UsesRequiresExcludedHack.insert(ActiveModule);
+
+    if (ShouldAddRequirement) {
+      // Add this feature.
+      ActiveModule->addRequirement(Feature, RequiredState, Map.LangOpts,
+                                   *Map.Target);
+    }
 
     if (!Tok.is(MMToken::Comma))
       break;
@@ -1657,9 +1708,16 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken,
       consumeToken();
     }
   }
+
   if (LeadingToken == MMToken::TextualKeyword)
     Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);
 
+  if (UsesRequiresExcludedHack.count(ActiveModule)) {
+    // Mark this header 'textual' (see doc comment for
+    // Module::UsesRequiresExcludedHack).
+    Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);
+  }
+
   if (LeadingToken != MMToken::HeaderKeyword) {
     if (!Tok.is(MMToken::HeaderKeyword)) {
       Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header)
@@ -1838,14 +1896,40 @@ void ModuleMapParser::parseUmbrellaDirDecl(SourceLocation UmbrellaLoc) {
     HadError = true;
     return;
   }
-  
+
+  if (UsesRequiresExcludedHack.count(ActiveModule)) {
+    // Mark this header 'textual' (see doc comment for
+    // ModuleMapParser::UsesRequiresExcludedHack). Although iterating over the
+    // directory is relatively expensive, in practice this only applies to the
+    // uncommonly used Tcl module on Darwin platforms.
+    std::error_code EC;
+    SmallVector<Module::Header, 6> Headers;
+    for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(), EC), E;
+         I != E && !EC; I.increment(EC)) {
+      if (const FileEntry *FE = SourceMgr.getFileManager().getFile(I->path())) {
+
+        Module::Header Header = {I->path(), FE};
+        Headers.push_back(std::move(Header));
+      }
+    }
+
+    // Sort header paths so that the pcm doesn't depend on iteration order.
+    llvm::array_pod_sort(Headers.begin(), Headers.end(),
+                         [](const Module::Header *A, const Module::Header *B) {
+                           return A->NameAsWritten.compare(B->NameAsWritten);
+                         });
+    for (auto &Header : Headers)
+      Map.addHeader(ActiveModule, std::move(Header), ModuleMap::TextualHeader);
+    return;
+  }
+
   if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {
     Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)
       << OwningModule->getFullModuleName();
     HadError = true;
     return;
-  } 
-  
+  }
+
   // Record this umbrella directory.
   Map.setUmbrellaDir(ActiveModule, Dir, DirName);
 }

diff --git a/test/Modules/Inputs/System/usr/include/assert.h b/test/Modules/Inputs/System/usr/include/assert.h
new file mode 100644 (file)
index 0000000..844e379
--- /dev/null
+++ b/test/Modules/Inputs/System/usr/include/assert.h
@@ -0,0 +1,2 @@
+// assert.h
+#define DARWIN_C_EXCLUDED 1

diff --git a/test/Modules/Inputs/System/usr/include/module.map b/test/Modules/Inputs/System/usr/include/module.map
index 9b2f3af2bacee53ef9f8fdfe64414f3af94bcc82..1d88ca380f49a50f783a37a9ccee381ece00ba63 100644 (file)
--- a/test/Modules/Inputs/System/usr/include/module.map
+++ b/test/Modules/Inputs/System/usr/include/module.map
@@ -30,3 +30,25 @@ module uses_other_constants {
   header "uses_other_constants.h"
   export *
 }
+
+module Darwin {
+  module C {
+    module excluded {
+      requires excluded
+      header "assert.h"
+    }
+  }
+}
+
+module Tcl {
+  module Private {
+    requires excluded
+    umbrella ""
+  }
+}
+
+module IOKit {
+  module avc {
+    requires cplusplus
+  }
+}

diff --git a/test/Modules/Inputs/System/usr/include/tcl-private/header.h b/test/Modules/Inputs/System/usr/include/tcl-private/header.h
new file mode 100644 (file)
index 0000000..0e8fb64
--- /dev/null
+++ b/test/Modules/Inputs/System/usr/include/tcl-private/header.h
@@ -0,0 +1,2 @@
+// tcl-private/header.h
+#define TCL_PRIVATE 1

diff --git a/test/Modules/darwin_specific_modulemap_hacks.m b/test/Modules/darwin_specific_modulemap_hacks.m
new file mode 100644 (file)
index 0000000..82fc697
--- /dev/null
+++ b/test/Modules/darwin_specific_modulemap_hacks.m
@@ -0,0 +1,22 @@
+// RUN: rm -rf %t
+// 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
+// expected-no-diagnostics
+
+@import Darwin.C.excluded; // no error, header is implicitly 'textual'
+@import Tcl.Private;       // no error, header is implicitly 'textual'
+@import IOKit.avc;         // no error, cplusplus requirement removed
+
+#if defined(DARWIN_C_EXCLUDED)
+#error assert.h should be textual
+#elif defined(TCL_PRIVATE)
+#error tcl-private/header.h should be textual
+#endif
+
+#import <assert.h>
+#import <tcl-private/header.h>
+
+#if !defined(DARWIN_C_EXCLUDED)
+#error assert.h missing
+#elif !defined(TCL_PRIVATE)
+#error tcl-private/header.h missing
+#endif