benlangmuir updated this revision to Diff 31354.
benlangmuir added a comment.
Sorry for the slow response, I somehow missed your original comment.
Add a more cohesive comment. I don't think we can usefully specify the SDK
range it affects until a fixed SDK is available (currently it's every SDK that
has module maps at all). I'd love to update the comment then. Ideally at that
point we could also check the SDK version before applying the hack, but I'm not
sure it's worth reading the SDK version plist file from disk just for that. We
already read this file when creating the module cache hash, but I'd like to
remove that at some point.
Repository:
rL LLVM
http://reviews.llvm.org/D11403
Files:
include/clang/Basic/Module.h
lib/Basic/Module.cpp
lib/Lex/ModuleMap.cpp
test/Modules/Inputs/System/usr/include/assert.h
test/Modules/Inputs/System/usr/include/module.map
test/Modules/Inputs/System/usr/include/tcl-private/header.h
test/Modules/darwin_specific_modulemap_hacks.m
Index: test/Modules/darwin_specific_modulemap_hacks.m
===================================================================
--- /dev/null
+++ 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 <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
Index: test/Modules/Inputs/System/usr/include/tcl-private/header.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/System/usr/include/tcl-private/header.h
@@ -0,0 +1,2 @@
+// tcl-private/header.h
+#define TCL_PRIVATE 1
Index: test/Modules/Inputs/System/usr/include/module.map
===================================================================
--- test/Modules/Inputs/System/usr/include/module.map
+++ test/Modules/Inputs/System/usr/include/module.map
@@ -30,3 +30,25 @@
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
+ }
+}
Index: test/Modules/Inputs/System/usr/include/assert.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/System/usr/include/assert.h
@@ -0,0 +1,2 @@
+// assert.h
+#define DARWIN_C_EXCLUDED 1
Index: lib/Lex/ModuleMap.cpp
===================================================================
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -1570,6 +1570,41 @@
: 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) {
+ if (Feature == "excluded" || Feature == "cplusplus") {
+ std::string FullName = M->getFullModuleName();
+ if (FullName == "Darwin.C.excluded" || FullName == "Tcl.Private") {
+ // We will mark the module contents non-modular. See doc comment for
+ // Module::UsesRequiresExcludedHack.
+ IsRequiresExcludedHack = true;
+ return false;
+ } else if (FullName == "IOKit.avc") {
+ // This module was mistakenly marked 'requires cplusplus' in older Darwin
+ // SDK versions. As a backwards compatibility hack, don't add the
+ // requirement.
+ return false;
+ }
+ }
+
+ return true;
+}
+
/// \brief Parse a requires declaration.
///
/// requires-declaration:
@@ -1605,9 +1640,16 @@
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);
+ ActiveModule->UsesRequiresExcludedHack |= IsRequiresExcludedHack;
+
+ if (ShouldAddRequirement) {
+ // Add this feature.
+ ActiveModule->addRequirement(Feature, RequiredState, Map.LangOpts,
+ *Map.Target);
+ }
if (!Tok.is(MMToken::Comma))
break;
@@ -1657,9 +1699,16 @@
consumeToken();
}
}
+
if (LeadingToken == MMToken::TextualKeyword)
Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);
+ if (ActiveModule->UsesRequiresExcludedHack) {
+ // 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 +1887,33 @@
HadError = true;
return;
}
-
+
+ if (ActiveModule->UsesRequiresExcludedHack) {
+ // Mark this header 'textual' (see doc comment for
+ // Module::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;
+ 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())) {
+ // FIXME: Taking the name from the FileEntry is unstable and can give
+ // different results depending on how we've previously named that file
+ // in this build.
+ Module::Header Header = {FE->getName(), FE};
+ Map.addHeader(ActiveModule, 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);
}
Index: lib/Basic/Module.cpp
===================================================================
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -32,7 +32,8 @@
IsFramework(IsFramework), IsExplicit(IsExplicit), IsSystem(false),
IsExternC(false), IsInferred(false), InferSubmodules(false),
InferExplicitSubmodules(false), InferExportWildcard(false),
- ConfigMacrosExhaustive(false), NameVisibility(Hidden) {
+ ConfigMacrosExhaustive(false), UsesRequiresExcludedHack(false),
+ NameVisibility(Hidden) {
if (Parent) {
if (!Parent->isAvailable())
IsAvailable = false;
Index: include/clang/Basic/Module.h
===================================================================
--- include/clang/Basic/Module.h
+++ include/clang/Basic/Module.h
@@ -200,6 +200,16 @@
/// built.
unsigned ConfigMacrosExhaustive : 1;
+ /// \brief Whether this 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.
+ unsigned UsesRequiresExcludedHack : 1;
+
/// \brief Describes the visibility of the various names within a
/// particular module.
enum NameVisibilityKind {
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
