On Thu, Aug 13, 2015 at 4:47 PM, Ben Langmuir <blangm...@apple.com> wrote:
> Yep, that should be it! > Great! -- Sean Silva > > On Aug 13, 2015, at 4:45 PM, Sean Silva <chisophu...@gmail.com> wrote: > > 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 <http://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 <http://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