This was the last thing blocking http://reviews.llvm.org/D10423 on your side, right?
-- Sean Silva On Thu, Aug 13, 2015 at 10:13 AM, Ben Langmuir via cfe-commits < cfe-commits@lists.llvm.org> wrote: > Author: benlangmuir > Date: Thu Aug 13 12:13:33 2015 > New Revision: 244912 > > URL: http://llvm.org/viewvc/llvm-project?rev=244912&view=rev > Log: > [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. > > Added: > cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h > cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/ > cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h > cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m > Modified: > cfe/trunk/include/clang/Basic/Module.h > cfe/trunk/lib/Basic/Module.cpp > cfe/trunk/lib/Lex/ModuleMap.cpp > cfe/trunk/test/Modules/Inputs/System/usr/include/module.map > > Modified: cfe/trunk/include/clang/Basic/Module.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=244912&r1=244911&r2=244912&view=diff > > ============================================================================== > --- cfe/trunk/include/clang/Basic/Module.h (original) > +++ cfe/trunk/include/clang/Basic/Module.h Thu Aug 13 12:13:33 2015 > @@ -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() { > > Modified: cfe/trunk/lib/Basic/Module.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=244912&r1=244911&r2=244912&view=diff > > ============================================================================== > --- cfe/trunk/lib/Basic/Module.cpp (original) > +++ cfe/trunk/lib/Basic/Module.cpp Thu Aug 13 12:13:33 2015 > @@ -139,6 +139,15 @@ std::string Module::getFullModuleName() > 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()}; > > Modified: cfe/trunk/lib/Lex/ModuleMap.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=244912&r1=244911&r2=244912&view=diff > > ============================================================================== > --- cfe/trunk/lib/Lex/ModuleMap.cpp (original) > +++ cfe/trunk/lib/Lex/ModuleMap.cpp Thu Aug 13 12:13:33 2015 > @@ -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::parseExternModuleD > : 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(MM > 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::parseUmbrellaDirDe > 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); > } > > Added: cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h?rev=244912&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h (added) > +++ cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h Thu Aug 13 > 12:13:33 2015 > @@ -0,0 +1,2 @@ > +// assert.h > +#define DARWIN_C_EXCLUDED 1 > > Modified: cfe/trunk/test/Modules/Inputs/System/usr/include/module.map > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/module.map?rev=244912&r1=244911&r2=244912&view=diff > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/System/usr/include/module.map (original) > +++ cfe/trunk/test/Modules/Inputs/System/usr/include/module.map Thu Aug 13 > 12:13:33 2015 > @@ -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 > + } > +} > > Added: > cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h?rev=244912&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h > (added) > +++ cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h > Thu Aug 13 12:13:33 2015 > @@ -0,0 +1,2 @@ > +// tcl-private/header.h > +#define TCL_PRIVATE 1 > > Added: cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m?rev=244912&view=auto > > ============================================================================== > --- cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m (added) > +++ cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m Thu Aug 13 > 12:13:33 2015 > @@ -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 > > > _______________________________________________ > cfe-commits mailing list > cfe-commits@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
