From 3476d7573690b664ca7346fbaf2b5cba90690fd1 Mon Sep 17 00:00:00 2001 From: Ben Langmuir Date: Thu, 13 Aug 2015 17:13:33 +0000 Subject: [PATCH] [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 --- include/clang/Basic/Module.h | 6 ++ lib/Basic/Module.cpp | 9 ++ lib/Lex/ModuleMap.cpp | 98 +++++++++++++++++-- .../Inputs/System/usr/include/assert.h | 2 + .../Inputs/System/usr/include/module.map | 22 +++++ .../System/usr/include/tcl-private/header.h | 2 + .../Modules/darwin_specific_modulemap_hacks.m | 22 +++++ 7 files changed, 154 insertions(+), 7 deletions(-) create mode 100644 test/Modules/Inputs/System/usr/include/assert.h create mode 100644 test/Modules/Inputs/System/usr/include/tcl-private/header.h create mode 100644 test/Modules/darwin_specific_modulemap_hacks.m diff --git a/include/clang/Basic/Module.h b/include/clang/Basic/Module.h index 1bc89257d3..e3060456df 100644 --- 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 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 4314b41eb3..4188cefc84 100644 --- 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 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 995e1370ed..a1c2d23fa3 100644 --- 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 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 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 index 0000000000..844e379091 --- /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 9b2f3af2ba..1d88ca380f 100644 --- 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 index 0000000000..0e8fb64a71 --- /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 index 0000000000..82fc697883 --- /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 +#import + +#if !defined(DARWIN_C_EXCLUDED) +#error assert.h missing +#elif !defined(TCL_PRIVATE) +#error tcl-private/header.h missing +#endif -- 2.40.0