Bigcheese updated this revision to Diff 405133.
Bigcheese added a comment.
Add testing of empty blocks and nested blocks and make the missing `{` error
clearer.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D118311/new/
https://reviews.llvm.org/D118311
Files:
clang/docs/Modules.rst
clang/include/clang/Basic/DiagnosticLexKinds.td
clang/include/clang/Basic/Module.h
clang/lib/Basic/Module.cpp
clang/lib/Lex/ModuleMap.cpp
clang/test/Modules/requires-block-errors.m
clang/test/Modules/requires-block.m
Index: clang/test/Modules/requires-block.m
===================================================================
--- /dev/null
+++ clang/test/Modules/requires-block.m
@@ -0,0 +1,73 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/include %t/A.m -verify
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/include %t/A.mm
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/include %t/B.m -verify
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/include %t/B.mm -verify
+
+//--- include/module.modulemap
+
+requires !cplusplus {
+ module A {
+ header "A.h"
+ }
+}
+
+requires cplusplus {
+ module A {
+ header "A.h"
+ header "A.hpp"
+ }
+}
+
+// Test empty blocks
+requires cplusplus {}
+requires !cplusplus {}
+
+// Test nested requires
+requires objc {
+ module B {
+ requires !cplusplus {
+ header "B.h"
+ }
+ requires cplusplus {
+ header "B.hpp"
+ }
+ }
+}
+
+//--- include/A.h
+
+typedef int A_t;
+
+//--- include/A.hpp
+
+using Adrgdrg_t = long;
+
+//--- include/B.h
+
+typedef int B_t;
+
+//--- include/B.hpp
+
+using Bdrgdrg_t = long;
+
+//--- A.m
+@import A;
+A_t a;
+Adrgdrg_t b; // expected-error {{unknown type name 'Adrgdrg_t'}}
+
+//--- A.mm
+@import A;
+A_t a;
+Adrgdrg_t b;
+
+//--- B.m
+@import B;
+B_t a;
+Bdrgdrg_t b; // expected-error {{unknown type name 'Bdrgdrg_t'}}
+
+//--- B.mm
+@import B;
+B_t a; // expected-error {{unknown type name 'B_t'}}
+Bdrgdrg_t b;
Index: clang/test/Modules/requires-block-errors.m
===================================================================
--- /dev/null
+++ clang/test/Modules/requires-block-errors.m
@@ -0,0 +1,32 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/missing-closing-brace %t/missing-closing-brace.m -verify
+// RUN: %clang_cc1 -fmodules-cache-path=%t/module-cache -fmodules -fimplicit-module-maps -I %t/missing-opening-brace %t/missing-opening-brace.m -verify
+
+//--- missing-closing-brace/module.modulemap
+
+requires !cplusplus {
+ module Pony {
+ header "Pony.h"
+ }
+
+//--- missing-closing-brace/Pony.h
+
+//--- missing-closing-brace.m
+// expected-error@module.modulemap:7 {{expected '}'}}
+// expected-note@module.modulemap:2 {{to match this '{'}}
+@import Pony // expected-error {{could not build module 'Pony'}}
+
+//--- missing-opening-brace/module.modulemap
+
+requires !cplusplus
+module Pony {
+ header "Pony.h"
+}
+
+//--- missing-opening-brace/Pony.h
+
+//--- missing-opening-brace.m
+// expected-error@module.modulemap:3 {{expected '{' to start requires block}}
+// expected-note@module.modulemap:2 {{for requires block declared here}}
+@import Pony // expected-error {{could not build module 'Pony'}}
Index: clang/lib/Lex/ModuleMap.cpp
===================================================================
--- clang/lib/Lex/ModuleMap.cpp
+++ clang/lib/Lex/ModuleMap.cpp
@@ -1468,10 +1468,19 @@
using ModuleId = SmallVector<std::pair<std::string, SourceLocation>, 2>;
+ struct RequiresFeature {
+ bool RequiredState;
+ std::string Feature;
+ };
+ using RequiresFeatureList = SmallVector<RequiresFeature, 2>;
+
bool parseModuleId(ModuleId &Id);
void parseModuleDecl();
+ void parseModuleMembers();
void parseExternModuleDecl();
- void parseRequiresDecl();
+ void parseRequiresFeatureList(RequiresFeatureList &RFL);
+ void parseRequiresDecl(bool TopLevel = false);
+ void parseRequiresBlockBody(const RequiresFeatureList &RFL);
void parseHeaderDecl(MMToken::TokenKind, SourceLocation LeadingLoc);
void parseUmbrellaDirDecl(SourceLocation UmbrellaLoc);
void parseExportDecl();
@@ -1825,6 +1834,7 @@
///
/// module-member:
/// requires-declaration
+/// requires-block-declaration
/// header-declaration
/// submodule-declaration
/// export-declaration
@@ -2030,6 +2040,38 @@
ActiveModule->ModuleMapIsPrivate)
diagnosePrivateModules(ExplicitLoc, FrameworkLoc);
+ parseModuleMembers();
+
+ if (Tok.is(MMToken::RBrace))
+ consumeToken();
+ else {
+ Diags.Report(Tok.getLocation(), diag::err_mmap_expected_rbrace);
+ Diags.Report(LBraceLoc, diag::note_mmap_lbrace_match);
+ HadError = true;
+ }
+
+ // If the active module is a top-level framework, and there are no link
+ // libraries, automatically link against the framework.
+ if (ActiveModule->IsFramework && !ActiveModule->isSubFramework() &&
+ ActiveModule->LinkLibraries.empty()) {
+ inferFrameworkLink(ActiveModule, Directory, SourceMgr.getFileManager());
+ }
+
+ // If the module meets all requirements but is still unavailable, mark the
+ // whole tree as unavailable to prevent it from building.
+ if (!ActiveModule->IsAvailable && !ActiveModule->IsUnimportable &&
+ ActiveModule->Parent) {
+ ActiveModule->getTopLevelModule()->markUnavailable(/*Unimportable=*/false);
+ ActiveModule->getTopLevelModule()->MissingHeaders.append(
+ ActiveModule->MissingHeaders.begin(),
+ ActiveModule->MissingHeaders.end());
+ }
+
+ // We're done parsing this module. Pop back to the previous module.
+ ActiveModule = PreviousActiveModule;
+}
+
+void ModuleMapParser::parseModuleMembers() {
bool Done = false;
do {
switch (Tok.Kind) {
@@ -2104,33 +2146,6 @@
break;
}
} while (!Done);
-
- if (Tok.is(MMToken::RBrace))
- consumeToken();
- else {
- Diags.Report(Tok.getLocation(), diag::err_mmap_expected_rbrace);
- Diags.Report(LBraceLoc, diag::note_mmap_lbrace_match);
- HadError = true;
- }
-
- // If the active module is a top-level framework, and there are no link
- // libraries, automatically link against the framework.
- if (ActiveModule->IsFramework && !ActiveModule->isSubFramework() &&
- ActiveModule->LinkLibraries.empty()) {
- inferFrameworkLink(ActiveModule, Directory, SourceMgr.getFileManager());
- }
-
- // If the module meets all requirements but is still unavailable, mark the
- // whole tree as unavailable to prevent it from building.
- if (!ActiveModule->IsAvailable && !ActiveModule->IsUnimportable &&
- ActiveModule->Parent) {
- ActiveModule->getTopLevelModule()->markUnavailable(/*Unimportable=*/false);
- ActiveModule->getTopLevelModule()->MissingHeaders.append(
- ActiveModule->MissingHeaders.begin(), ActiveModule->MissingHeaders.end());
- }
-
- // We're done parsing this module. Pop back to the previous module.
- ActiveModule = PreviousActiveModule;
}
/// Parse an extern module declaration.
@@ -2211,10 +2226,7 @@
return true;
}
-/// Parse a requires declaration.
-///
-/// requires-declaration:
-/// 'requires' feature-list
+/// Parse a requires or requires block feature-list.
///
/// feature-list:
/// feature ',' feature-list
@@ -2222,13 +2234,7 @@
///
/// feature:
/// '!'[opt] identifier
-void ModuleMapParser::parseRequiresDecl() {
- assert(Tok.is(MMToken::RequiresKeyword));
-
- // Parse 'requires' keyword.
- consumeToken();
-
- // Parse the feature-list.
+void ModuleMapParser::parseRequiresFeatureList(RequiresFeatureList &RFL) {
do {
bool RequiredState = true;
if (Tok.is(MMToken::Exclaim)) {
@@ -2246,25 +2252,122 @@
std::string Feature = std::string(Tok.getString());
consumeToken();
+ RFL.push_back({RequiredState, Feature});
+
+ if (!Tok.is(MMToken::Comma))
+ break;
+
+ // Consume the comma.
+ consumeToken();
+ } while (true);
+}
+
+/// Parse a requires or requires block declaration.
+///
+/// requires-declaration:
+/// 'requires' feature-list
+///
+/// requires-block-declaration:
+/// 'requires' feature-list '{' requires-block-body '}'
+///
+void ModuleMapParser::parseRequiresDecl(bool TopLevel) {
+ assert(Tok.is(MMToken::RequiresKeyword));
+
+ // Parse 'requires' keyword.
+ SourceLocation RequiresLoc = consumeToken();
+
+ // Parse the feature-list.
+ RequiresFeatureList RFL;
+ parseRequiresFeatureList(RFL);
+
+ if (Tok.is(MMToken::LBrace)) {
+ return parseRequiresBlockBody(RFL);
+ } else if (TopLevel) {
+ Diags.Report(Tok.getLocation(), diag::err_mmap_expected_lbrace_requires);
+ Diags.Report(RequiresLoc, diag::note_mmap_expected_lbrace_requires);
+ HadError = true;
+ return;
+ }
+
+ for (const RequiresFeature &F : RFL) {
bool IsRequiresExcludedHack = false;
bool ShouldAddRequirement =
- shouldAddRequirement(ActiveModule, Feature, IsRequiresExcludedHack);
-
+ shouldAddRequirement(ActiveModule, F.Feature, IsRequiresExcludedHack);
if (IsRequiresExcludedHack)
UsesRequiresExcludedHack.insert(ActiveModule);
-
if (ShouldAddRequirement) {
// Add this feature.
- ActiveModule->addRequirement(Feature, RequiredState, Map.LangOpts,
+ ActiveModule->addRequirement(F.Feature, F.RequiredState, Map.LangOpts,
*Map.Target);
}
+ }
+}
- if (!Tok.is(MMToken::Comma))
- break;
+/// Parse a requires block body.
+///
+/// requires-block-body:
+/// module-declaration*
+/// module-member*
+///
+void ModuleMapParser::parseRequiresBlockBody(const RequiresFeatureList &RFL) {
+ assert(Tok.is(MMToken::LBrace));
- // Consume the comma.
- consumeToken();
- } while (true);
+ // Check if the feature list is satisfied.
+ bool Satisfied = true;
+ for (const RequiresFeature &F : RFL) {
+ if (Module::hasFeature(F.Feature, Map.LangOpts, *Map.Target) !=
+ F.RequiredState)
+ Satisfied = false;
+ }
+
+ // Consume the {
+ SourceLocation LBraceLoc = consumeToken();
+
+ // Brace balanaced skipping of tokens until the end of the block.
+ if (!Satisfied) {
+ int BraceDepth = 1;
+ while (BraceDepth != 0) {
+ switch (Tok.Kind) {
+ case MMToken::LBrace:
+ ++BraceDepth;
+ break;
+ case MMToken::RBrace:
+ --BraceDepth;
+ break;
+ case MMToken::EndOfFile:
+ Diags.Report(Tok.getLocation(), diag::err_mmap_expected_rbrace);
+ Diags.Report(LBraceLoc, diag::note_mmap_lbrace_match);
+ BraceDepth = 0;
+ HadError = true;
+ break;
+ default:
+ break;
+ }
+ consumeToken();
+ }
+
+ return;
+ }
+
+ while (true) {
+ switch (Tok.Kind) {
+ case MMToken::RBrace:
+ consumeToken();
+ return;
+ case MMToken::EndOfFile:
+ Diags.Report(Tok.getLocation(), diag::err_mmap_expected_rbrace);
+ Diags.Report(LBraceLoc, diag::note_mmap_lbrace_match);
+ HadError = true;
+ return;
+ default:
+ // Parse the content.
+ if (!ActiveModule) {
+ parseModuleDecl();
+ } else {
+ parseModuleMembers();
+ }
+ }
+ }
}
/// Parse a header declaration.
@@ -2951,6 +3054,7 @@
///
/// module-map-file:
/// module-declaration*
+/// requires-block-declaration*
bool ModuleMapParser::parseModuleMapFile() {
do {
switch (Tok.Kind) {
@@ -2964,6 +3068,10 @@
parseModuleDecl();
break;
+ case MMToken::RequiresKeyword:
+ parseRequiresDecl(true);
+ break;
+
case MMToken::Comma:
case MMToken::ConfigMacros:
case MMToken::Conflict:
@@ -2980,7 +3088,6 @@
case MMToken::PrivateKeyword:
case MMToken::RBrace:
case MMToken::RSquare:
- case MMToken::RequiresKeyword:
case MMToken::Star:
case MMToken::StringLiteral:
case MMToken::IntegerLiteral:
Index: clang/lib/Basic/Module.cpp
===================================================================
--- clang/lib/Basic/Module.cpp
+++ clang/lib/Basic/Module.cpp
@@ -96,10 +96,8 @@
return PlatformEnv == Feature;
}
-/// Determine whether a translation unit built using the current
-/// language options has the given feature.
-static bool hasFeature(StringRef Feature, const LangOptions &LangOpts,
- const TargetInfo &Target) {
+bool Module::hasFeature(StringRef Feature, const LangOptions &LangOpts,
+ const TargetInfo &Target) {
bool HasFeature = llvm::StringSwitch<bool>(Feature)
.Case("altivec", LangOpts.AltiVec)
.Case("blocks", LangOpts.Blocks)
Index: clang/include/clang/Basic/Module.h
===================================================================
--- clang/include/clang/Basic/Module.h
+++ clang/include/clang/Basic/Module.h
@@ -244,6 +244,11 @@
/// will be false to indicate that this (sub)module is not available.
SmallVector<Requirement, 2> Requirements;
+ /// Determine whether a translation unit built using the current
+ /// language options has the given feature.
+ static bool hasFeature(StringRef Feature, const LangOptions &LangOpts,
+ const TargetInfo &Target);
+
/// A module with the same name that shadows this module.
Module *ShadowingModule = nullptr;
Index: clang/include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- clang/include/clang/Basic/DiagnosticLexKinds.td
+++ clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -718,6 +718,10 @@
def err_mmap_expected_module : Error<"expected module declaration">;
def err_mmap_expected_module_name : Error<"expected module name">;
def err_mmap_expected_lbrace : Error<"expected '{' to start module '%0'">;
+def err_mmap_expected_lbrace_requires : Error<
+ "expected '{' to start requires block">;
+def note_mmap_expected_lbrace_requires : Note<
+ "for requires block declared here">;
def err_mmap_expected_rbrace : Error<"expected '}'">;
def note_mmap_lbrace_match : Note<"to match this '{'">;
def err_mmap_expected_rsquare : Error<"expected ']' to close attribute">;
Index: clang/docs/Modules.rst
===================================================================
--- clang/docs/Modules.rst
+++ clang/docs/Modules.rst
@@ -470,6 +470,7 @@
*module-map-file*:
*module-declaration**
+ *requires-block**
Within a module map file, modules are referred to by a *module-id*, which uses periods to separate each part of a module's name:
@@ -516,6 +517,7 @@
*module-member*:
*requires-declaration*
+ *requires-block-declaration*
*header-declaration*
*umbrella-dir-declaration*
*submodule-declaration*
@@ -622,6 +624,38 @@
}
}
+Requires block declaration
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+A requires block declaration allows conditional inclusion of other declarations within a module map.
+
+.. parsed-literal::
+
+ *requires-block-declaration*:
+ ``requires`` *feature-list* '{' *requires-block-body* '}'
+
+ *requires-block-body*:
+ *module-declaration**
+ *module-member**
+
+The syntax is the same as a *requires-declaration*, except that it is followed by a block. If the *feature-list* isn't satisfied, then the contents of the block is ignored and skipped over. If the *feature-list* is satisfied, then the contents of the requires block are parsed as if the requires block was not present.
+
+This differs from a *requires-declaration* in that it is not an error to import a module from within an unsatisfied *requires-block-declaration* as long as another declaration of the module exists.
+
+**Example:** The ``std`` module can be extended to also include C++ and C++11 headers without creating a submodule using a *requires-block-declaration*:
+
+.. parsed-literal::
+
+ module std {
+ // C standard library...
+
+ requires cplusplus {
+ header "vector"
+ }
+ requires cplusplus11 {
+ header "type_traits"
+ }
+ }
+
Header declaration
~~~~~~~~~~~~~~~~~~
A header declaration specifies that a particular header is associated with the enclosing module.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
