Skip to content

Commit

Permalink
[Modules] Add Darwin-specific compatibility module map parsing hacks
Browse files Browse the repository at this point in the history
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.

llvm-svn: 244912
  • Loading branch information
benlangmuir committed Aug 13, 2015
1 parent a747179 commit 7ff2914
Show file tree
Hide file tree
Showing 7 changed files with 154 additions and 7 deletions.
6 changes: 6 additions & 0 deletions clang/include/clang/Basic/Module.h
Original file line number Diff line number Diff line change
Expand Up @@ -356,6 +356,12 @@ class Module {
/// 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() {
Expand Down
9 changes: 9 additions & 0 deletions clang/lib/Basic/Module.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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()};
Expand Down
98 changes: 91 additions & 7 deletions clang/lib/Lex/ModuleMap.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down
2 changes: 2 additions & 0 deletions clang/test/Modules/Inputs/System/usr/include/assert.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// assert.h
#define DARWIN_C_EXCLUDED 1
22 changes: 22 additions & 0 deletions clang/test/Modules/Inputs/System/usr/include/module.map
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
// tcl-private/header.h
#define TCL_PRIVATE 1
22 changes: 22 additions & 0 deletions clang/test/Modules/darwin_specific_modulemap_hacks.m
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit 7ff2914

Please sign in to comment.