[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [PATCH] D11403: [Modules] Add Darwin-specific compatibility module map parsing hacks
From: Ben Langmuir via cfe-commits <cfe-commits () lists ! llvm ! org>
Date: 2015-08-12 15:57:31
Message-ID: cc74744d0a9caf0c824b7bf48d4e159f () localhost ! localdomain
[Download RAW message or body]
benlangmuir updated this revision to Diff 31947.
benlangmuir added a comment.
Changes per review. I still need to check performance of code-completing i=
mport statements to make sure the extra directory iteration isn't a big reg=
ression.
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
["D11403.31947.patch" (text/x-patch)]
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
@@ -1015,7 +1015,18 @@
/// \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 +1581,38 @@
: 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->fullModuleNameEqual(DarwinCExcluded) ||
+ M->fullModuleNameEqual(TclPrivate))) {
+ IsRequiresExcludedHack = true;
+ return false;
+ } else if (Feature == "cplusplus" && M->fullModuleNameEqual(IOKitAVC)) {
+ return false;
+ }
+
+ return true;
+}
+
/// \brief Parse a requires declaration.
///
/// requires-declaration:
@@ -1605,9 +1648,18 @@
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 +1709,16 @@
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 +1897,40 @@
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);
}
Index: lib/Basic/Module.cpp
===================================================================
--- lib/Basic/Module.cpp
+++ lib/Basic/Module.cpp
@@ -138,6 +138,15 @@
return Result;
}
+bool Module::fullModuleNameEqual(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()};
Index: include/clang/Basic/Module.h
===================================================================
--- include/clang/Basic/Module.h
+++ include/clang/Basic/Module.h
@@ -356,6 +356,12 @@
/// 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 fullModuleNameEqual(ArrayRef<StringRef> nameParts) const;
+
/// \brief Retrieve the top-level module for this (sub)module, which may
/// be this module.
Module *getTopLevelModule() {
[Attachment #4 (text/plain)]
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic