From a3feee2ad64b6604bcb35c27a35307bf470accd1 Mon Sep 17 00:00:00 2001 From: Richard Smith Date: Mon, 28 Oct 2013 22:18:19 +0000 Subject: [PATCH] Allow a new syntax in a module requires-declaration: requires ! feature The purpose of this is to allow (for instance) the module map for /usr/include to exclude and when building in C++ (these headers are instead provided by the C++ standard library in this case, and the glibc C header would otherwise try to include , resulting in a module cycle). llvm-svn: 193549 --- clang/docs/Modules.rst | 7 ++++-- .../include/clang/Basic/DiagnosticFrontendKinds.td | 3 ++- clang/include/clang/Basic/Module.h | 26 +++++++++++++------- clang/lib/Basic/Module.cpp | 28 +++++++++++++--------- clang/lib/Frontend/CompilerInstance.cpp | 6 ++--- clang/lib/Frontend/FrontendActions.cpp | 6 ++--- clang/lib/Lex/ModuleMap.cpp | 22 ++++++++++++++--- clang/lib/Serialization/ASTReader.cpp | 2 +- clang/lib/Serialization/ASTWriter.cpp | 9 +++---- clang/test/Index/index-module.m | 2 ++ .../DependsOnModule.framework/Headers/not_cxx.h | 1 + .../DependsOnModule.framework/Headers/not_objc.h | 1 + .../Inputs/DependsOnModule.framework/module.map | 8 +++++++ clang/test/Modules/requires.m | 3 ++- clang/test/Modules/requires.mm | 6 +++++ 15 files changed, 92 insertions(+), 38 deletions(-) create mode 100644 clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_cxx.h create mode 100644 clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_objc.h create mode 100644 clang/test/Modules/requires.mm diff --git a/clang/docs/Modules.rst b/clang/docs/Modules.rst index bcb71c2..9fb4c77 100644 --- a/clang/docs/Modules.rst +++ b/clang/docs/Modules.rst @@ -320,9 +320,12 @@ A *requires-declaration* specifies the requirements that an importing translatio ``requires`` *feature-list* *feature-list*: - *identifier* (',' *identifier*)* + *feature* (',' *feature*)* + + *feature*: + ``!``:sub:`opt` *identifier* -The requirements clause allows specific modules or submodules to specify that they are only accessible with certain language dialects or on certain platforms. The feature list is a set of identifiers, defined below. If any of the features is not available in a given translation unit, that translation unit shall not import the module. +The requirements clause allows specific modules or submodules to specify that they are only accessible with certain language dialects or on certain platforms. The feature list is a set of identifiers, defined below. If any of the features is not available in a given translation unit, that translation unit shall not import the module. The optional ``!`` indicates that a feature is incompatible with the module. The following features are defined: diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index dc80af8..bcf3c41 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -136,7 +136,8 @@ def err_no_submodule_suggest : Error< "no submodule named %0 in module '%1'; did you mean '%2'?">; def warn_missing_submodule : Warning<"missing submodule '%0'">, InGroup; -def err_module_unavailable : Error<"module '%0' requires feature '%1'">; +def err_module_unavailable : Error< + "module '%0' %select{is incompatible with|requires}1 feature '%2'">; def warn_module_config_macro_undef : Warning< "%select{definition|#undef}0 of configuration macro '%1' has no effect on " "the import of '%2'; pass '%select{-D%1=...|-U%1}0' on the command line " diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index e6913d8..027e217 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -38,6 +38,7 @@ class FileEntry; class FileManager; class LangOptions; class TargetInfo; +class IdentifierInfo; /// \brief Describes the name of a module. typedef SmallVector, 2> ModuleId; @@ -89,12 +90,15 @@ public: /// \brief The headers that are private to this module. llvm::SmallVector PrivateHeaders; + /// \brief An individual requirement: a feature name and a flag indicating + /// the required state of that feature. + typedef std::pair Requirement; + /// \brief The set of language features required to use this module. /// - /// If any of these features is not present, the \c IsAvailable bit - /// will be false to indicate that this (sub)module is not - /// available. - SmallVector Requires; + /// If any of these requirements are not available, the \c IsAvailable bit + /// will be false to indicate that this (sub)module is not available. + SmallVector Requirements; /// \brief Whether this module is available in the current /// translation unit. @@ -267,12 +271,12 @@ public: /// /// \param Target The target options used for the current translation unit. /// - /// \param Feature If this module is unavailable, this parameter - /// will be set to one of the features that is required for use of - /// this module (but is not available). + /// \param Req If this module is unavailable, this parameter + /// will be set to one of the requirements that is not met for use of + /// this module. bool isAvailable(const LangOptions &LangOpts, const TargetInfo &Target, - StringRef &Feature) const; + Requirement &Req) const; /// \brief Determine whether this module is a submodule. bool isSubModule() const { return Parent != 0; } @@ -366,12 +370,16 @@ public: /// \param Feature The feature that is required by this module (and /// its submodules). /// + /// \param RequiredState The required state of this feature: \c true + /// if it must be present, \c false if it must be absent. + /// /// \param LangOpts The set of language options that will be used to /// evaluate the availability of this feature. /// /// \param Target The target options that will be used to evaluate the /// availability of this feature. - void addRequirement(StringRef Feature, const LangOptions &LangOpts, + void addRequirement(StringRef Feature, bool RequiredState, + const LangOptions &LangOpts, const TargetInfo &Target); /// \brief Find the submodule with the given name. diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 8c35b34..4815377 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -11,6 +11,7 @@ // code. // //===----------------------------------------------------------------------===// + #include "clang/Basic/Module.h" #include "clang/Basic/FileManager.h" #include "clang/Basic/LangOptions.h" @@ -20,6 +21,7 @@ #include "llvm/ADT/StringSwitch.h" #include "llvm/Support/ErrorHandling.h" #include "llvm/Support/raw_ostream.h" + using namespace clang; Module::Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent, @@ -65,16 +67,17 @@ static bool hasFeature(StringRef Feature, const LangOptions &LangOpts, .Default(Target.hasFeature(Feature)); } -bool +bool Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target, - StringRef &Feature) const { + Requirement &Req) const { if (IsAvailable) return true; for (const Module *Current = this; Current; Current = Current->Parent) { - for (unsigned I = 0, N = Current->Requires.size(); I != N; ++I) { - if (!hasFeature(Current->Requires[I], LangOpts, Target)) { - Feature = Current->Requires[I]; + for (unsigned I = 0, N = Current->Requirements.size(); I != N; ++I) { + if (hasFeature(Current->Requirements[I].first, LangOpts, Target) != + Current->Requirements[I].second) { + Req = Current->Requirements[I]; return false; } } @@ -143,12 +146,13 @@ ArrayRef Module::getTopHeaders(FileManager &FileMgr) { return llvm::makeArrayRef(TopHeaders.begin(), TopHeaders.end()); } -void Module::addRequirement(StringRef Feature, const LangOptions &LangOpts, +void Module::addRequirement(StringRef Feature, bool RequiredState, + const LangOptions &LangOpts, const TargetInfo &Target) { - Requires.push_back(Feature); + Requirements.push_back(Requirement(Feature, RequiredState)); // If this feature is currently available, we're done. - if (hasFeature(Feature, LangOpts, Target)) + if (hasFeature(Feature, LangOpts, Target) == RequiredState) return; if (!IsAvailable) @@ -275,13 +279,15 @@ void Module::print(raw_ostream &OS, unsigned Indent) const { OS << " {\n"; - if (!Requires.empty()) { + if (!Requirements.empty()) { OS.indent(Indent + 2); OS << "requires "; - for (unsigned I = 0, N = Requires.size(); I != N; ++I) { + for (unsigned I = 0, N = Requirements.size(); I != N; ++I) { if (I) OS << ", "; - OS << Requires[I]; + if (!Requirements[I].second) + OS << "!"; + OS << Requirements[I].first; } OS << "\n"; } diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 5d3fd45..eccb94c 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1359,11 +1359,11 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, } // Check whether this module is available. - StringRef Feature; - if (!Module->isAvailable(getLangOpts(), getTarget(), Feature)) { + clang::Module::Requirement Requirement; + if (!Module->isAvailable(getLangOpts(), getTarget(), Requirement)) { getDiagnostics().Report(ImportLoc, diag::err_module_unavailable) << Module->getFullModuleName() - << Feature + << Requirement.second << Requirement.first << SourceRange(Path.front().second, Path.back().second); LastModuleImportLoc = ImportLoc; LastModuleImportResult = ModuleLoadResult(); diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index 81eb642..a3ab1be 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -246,11 +246,11 @@ bool GenerateModuleAction::BeginSourceFileAction(CompilerInstance &CI, } // Check whether we can build this module at all. - StringRef Feature; - if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Feature)) { + clang::Module::Requirement Requirement; + if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement)) { CI.getDiagnostics().Report(diag::err_module_unavailable) << Module->getFullModuleName() - << Feature; + << Requirement.second << Requirement.first; return false; } diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 54007c7..f4dfa12 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -773,6 +773,7 @@ namespace clang { EndOfFile, HeaderKeyword, Identifier, + Exclaim, ExcludeKeyword, ExplicitKeyword, ExportKeyword, @@ -967,6 +968,10 @@ retry: Tok.Kind = MMToken::Star; break; + case tok::exclaim: + Tok.Kind = MMToken::Exclaim; + break; + case tok::string_literal: { if (LToken.hasUDSuffix()) { Diags.Report(LToken.getLocation(), diag::err_invalid_string_udl); @@ -1392,8 +1397,11 @@ void ModuleMapParser::parseExternModuleDecl() { /// 'requires' feature-list /// /// feature-list: -/// identifier ',' feature-list -/// identifier +/// feature ',' feature-list +/// feature +/// +/// feature: +/// '!'[opt] identifier void ModuleMapParser::parseRequiresDecl() { assert(Tok.is(MMToken::RequiresKeyword)); @@ -1402,6 +1410,12 @@ void ModuleMapParser::parseRequiresDecl() { // Parse the feature-list. do { + bool RequiredState = true; + if (Tok.is(MMToken::Exclaim)) { + RequiredState = false; + consumeToken(); + } + if (!Tok.is(MMToken::Identifier)) { Diags.Report(Tok.getLocation(), diag::err_mmap_expected_feature); HadError = true; @@ -1413,7 +1427,8 @@ void ModuleMapParser::parseRequiresDecl() { consumeToken(); // Add this feature. - ActiveModule->addRequirement(Feature, Map.LangOpts, *Map.Target); + ActiveModule->addRequirement(Feature, RequiredState, + Map.LangOpts, *Map.Target); if (!Tok.is(MMToken::Comma)) break; @@ -2077,6 +2092,7 @@ bool ModuleMapParser::parseModuleMapFile() { case MMToken::Comma: case MMToken::ConfigMacros: case MMToken::Conflict: + case MMToken::Exclaim: case MMToken::ExcludeKeyword: case MMToken::ExportKeyword: case MMToken::HeaderKeyword: diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 7e68656..57278d5 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3912,7 +3912,7 @@ bool ASTReader::ReadSubmoduleBlock(ModuleFile &F) { if (!CurrentModule) break; - CurrentModule->addRequirement(Blob, Context.getLangOpts(), + CurrentModule->addRequirement(Blob, Record[0], Context.getLangOpts(), Context.getTargetInfo()); break; } diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 3d14512..10957a2 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -2278,7 +2278,8 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { Abbrev = new BitCodeAbbrev(); Abbrev->Add(BitCodeAbbrevOp(SUBMODULE_REQUIRES)); - Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Feature + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Fixed, 1)); // State + Abbrev->Add(BitCodeAbbrevOp(BitCodeAbbrevOp::Blob)); // Feature unsigned RequiresAbbrev = Stream.EmitAbbrev(Abbrev); Abbrev = new BitCodeAbbrev(); @@ -2342,12 +2343,12 @@ void ASTWriter::WriteSubmodules(Module *WritingModule) { Stream.EmitRecordWithBlob(DefinitionAbbrev, Record, Mod->Name); // Emit the requirements. - for (unsigned I = 0, N = Mod->Requires.size(); I != N; ++I) { + for (unsigned I = 0, N = Mod->Requirements.size(); I != N; ++I) { Record.clear(); Record.push_back(SUBMODULE_REQUIRES); + Record.push_back(Mod->Requirements[I].second); Stream.EmitRecordWithBlob(RequiresAbbrev, Record, - Mod->Requires[I].data(), - Mod->Requires[I].size()); + Mod->Requirements[I].first); } // Emit the umbrella header, if there is one. diff --git a/clang/test/Index/index-module.m b/clang/test/Index/index-module.m index 77dee98..d695313 100644 --- a/clang/test/Index/index-module.m +++ b/clang/test/Index/index-module.m @@ -21,11 +21,13 @@ int glob; // CHECK-DMOD-NEXT: [ppIncludedFile]: [[DMOD_MODULE_H:.*/Modules/Inputs/DependsOnModule\.framework[/\\]Headers[/\\]DependsOnModule\.h]] | {{.*}} | hash loc: // CHECK-DMOD-NEXT: [ppIncludedFile]: {{.*}}/Modules/Inputs/Module.framework{{[/\\]}}Headers{{[/\\]}}Module.h | name: "Module/Module.h" | hash loc: {{.*}}/Modules/Inputs/DependsOnModule.framework{{[/\\]}}Headers{{[/\\]}}DependsOnModule.h:1:1 | isImport: 0 | isAngled: 1 | isModule: 1 // CHECK-DMOD-NEXT: [ppIncludedFile]: [[DMOD_OTHER_H:.*/Modules/Inputs/DependsOnModule\.framework[/\\]Headers[/\\]other\.h]] | {{.*}} | hash loc: +// CHECK-DMOD-NEXT: [ppIncludedFile]: [[DMOD_NOT_CXX_H:.*/Modules/Inputs/DependsOnModule\.framework[/\\]Headers[/\\]not_cxx\.h]] | {{.*}} | hash loc: // CHECK-DMOD-NEXT: [ppIncludedFile]: [[DMOD_SUB_H:.*/Modules/Inputs/DependsOnModule\.framework[/\\]Frameworks[/\\]SubFramework\.framework[/\\]Headers[/\\]SubFramework\.h]] | {{.*}} | hash loc: // CHECK-DMOD-NEXT: [ppIncludedFile]: [[DMOD_SUB_OTHER_H:.*/Modules/Inputs/DependsOnModule.framework[/\\]Frameworks/SubFramework\.framework/Headers/Other\.h]] | name: "SubFramework/Other.h" | hash loc: [[DMOD_SUB_H]]:1:1 | isImport: 0 | isAngled: 0 // CHECK-DMOD-NEXT: [ppIncludedFile]: [[DMOD_PRIVATE_H:.*/Modules/Inputs/DependsOnModule.framework[/\\]PrivateHeaders[/\\]DependsOnModulePrivate.h]] | {{.*}} | hash loc: // CHECK-DMOD-NEXT: [importedASTFile]: {{.*}}.cache{{[/\\]}}Module.pcm | loc: [[DMOD_MODULE_H]]:1:2 | name: "Module" | isImplicit: 1 // CHECK-DMOD-NEXT: [indexDeclaration]: kind: variable | name: depends_on_module_other | {{.*}} | loc: [[DMOD_OTHER_H]]:1:5 +// CHECK-DMOD-NEXT: [indexDeclaration]: kind: variable | name: template | {{.*}} | loc: [[DMOD_NOT_CXX_H]]:1:12 // CHECK-DMOD-NEXT: [importedASTFile]: {{.*}}.cache/DependsOnModule.pcm | loc: {{.*}}SubFramework.h:1:2 | name: "DependsOnModule.SubFramework.Other" | isImplicit: 1 // CHECK-DMOD-NEXT: [indexDeclaration]: kind: variable | name: sub_framework | {{.*}} | loc: [[DMOD_SUB_H]]:2:8 // CHECK-DMOD-NEXT: [indexDeclaration]: kind: variable | name: sub_framework_other | {{.*}} | loc: [[DMOD_SUB_OTHER_H]]:1:9 diff --git a/clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_cxx.h b/clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_cxx.h new file mode 100644 index 0000000..c29a921 --- /dev/null +++ b/clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_cxx.h @@ -0,0 +1 @@ +extern int template; diff --git a/clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_objc.h b/clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_objc.h new file mode 100644 index 0000000..f63975a --- /dev/null +++ b/clang/test/Modules/Inputs/DependsOnModule.framework/Headers/not_objc.h @@ -0,0 +1 @@ +int NSObject; diff --git a/clang/test/Modules/Inputs/DependsOnModule.framework/module.map b/clang/test/Modules/Inputs/DependsOnModule.framework/module.map index 2a3dd80..5a1caca 100644 --- a/clang/test/Modules/Inputs/DependsOnModule.framework/module.map +++ b/clang/test/Modules/Inputs/DependsOnModule.framework/module.map @@ -8,6 +8,14 @@ framework module DependsOnModule { requires cplusplus header "cxx_other.h" } + explicit module NotCXX { + requires !cplusplus + header "not_cxx.h" + } + explicit module NotObjC { + requires !objc + header "not_objc.h" + } explicit framework module SubFramework { umbrella header "SubFramework.h" diff --git a/clang/test/Modules/requires.m b/clang/test/Modules/requires.m index 83b524d..b8b2c55 100644 --- a/clang/test/Modules/requires.m +++ b/clang/test/Modules/requires.m @@ -2,4 +2,5 @@ // RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -F %S/Inputs %s -verify @import DependsOnModule.CXX; // expected-error{{module 'DependsOnModule.CXX' requires feature 'cplusplus'}} - +@import DependsOnModule.NotCXX; +@import DependsOnModule.NotObjC; // expected-error{{module 'DependsOnModule.NotObjC' is incompatible with feature 'objc'}} diff --git a/clang/test/Modules/requires.mm b/clang/test/Modules/requires.mm new file mode 100644 index 0000000..736f2fa --- /dev/null +++ b/clang/test/Modules/requires.mm @@ -0,0 +1,6 @@ +// RUN: rm -rf %t +// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -F %S/Inputs %s -verify + +@import DependsOnModule.CXX; +@import DependsOnModule.NotCXX; // expected-error{{module 'DependsOnModule.NotCXX' is incompatible with feature 'cplusplus'}} +@import DependsOnModule.NotObjC; // expected-error{{module 'DependsOnModule.NotObjC' is incompatible with feature 'objc'}} -- 2.7.4