r244912 - [Modules] Add Darwin-specific compatibility module map parsing hacks
Ben Langmuir via cfe-commits
cfe-commits at lists.llvm.org
Thu Aug 13 16:47:28 PDT 2015
Yep, that should be it!
> On Aug 13, 2015, at 4:45 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
> This was the last thing blocking http://reviews.llvm.org/D10423 <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 at lists.llvm.org <mailto:cfe-commits at 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 <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 <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 <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 <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 <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 <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 <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 <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
> +
> + at import Darwin.C.excluded; // no error, header is implicitly 'textual'
> + at import Tcl.Private; // no error, header is implicitly 'textual'
> + at 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 at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150813/45980e17/attachment-0001.html>
More information about the cfe-commits
mailing list