Ben:
- Added better source location.
- Updated testing. I renamed to auto-import-unavailable.cpp and expanded
testing as you asked for. Before, I was piggybacking on missing-header.m and
related files, but since that is a different code path (and has different
requirements and diagnostics), I thought it was better to keep the test
separate and self-contained.
- also emit note_implicit_top_level_module_import_here on missing requirement.
Richard:
- available-is-better.cpp and the changes to isBetterKnownHeader to take into
account availability.
Does this look okay?
Ben: hopefully the present patch is complete enough for you to start
figuring out exactly what hacks to add and where.
http://reviews.llvm.org/D10423
Files:
include/clang/Basic/DiagnosticCommonKinds.td
include/clang/Basic/DiagnosticFrontendKinds.td
include/clang/Basic/DiagnosticLexKinds.td
lib/Lex/ModuleMap.cpp
lib/Lex/PPDirectives.cpp
test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h
test/Modules/Inputs/auto-import-unavailable/missing_requirement.h
test/Modules/Inputs/auto-import-unavailable/module.modulemap
test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h
test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h
test/Modules/Inputs/available-is-better/available-is-better.h
test/Modules/Inputs/available-is-better/module.modulemap
test/Modules/Inputs/declare-use/module.map
test/Modules/Inputs/macro-reexport/module.modulemap
test/Modules/auto-import-unavailable.cpp
test/Modules/available-is-better.cpp
Index: test/Modules/available-is-better.cpp
===================================================================
--- /dev/null
+++ test/Modules/available-is-better.cpp
@@ -0,0 +1,5 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -x c++ -Rmodule-build -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/available-is-better %s 2>&1 | FileCheck %s
+
+#include "available-is-better.h"
+// CHECK: remark: building module
Index: test/Modules/auto-import-unavailable.cpp
===================================================================
--- /dev/null
+++ test/Modules/auto-import-unavailable.cpp
@@ -0,0 +1,47 @@
+// RUN: rm -rf %t
+// RUN: not %clang_cc1 -x c++ -Rmodule-build -DMISSING_HEADER -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=MISSING-HEADER
+// RUN: %clang_cc1 -x c++ -Rmodule-build -DNONREQUIRED_MISSING_HEADER -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=NONREQUIRED-MISSING-HEADER
+// RUN: not %clang_cc1 -x c++ -Rmodule-build -DMISSING_REQUIREMENT -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I %S/Inputs/auto-import-unavailable %s 2>&1 | FileCheck %s --check-prefix=MISSING-REQUIREMENT
+
+#ifdef MISSING_HEADER
+
+// Even if the header we ask for is not missing, if the top-level module
+// containing it has a missing header, then the whole top-level is
+// unavailable and we issue an error.
+
+// MISSING-HEADER: module.modulemap:2:27: error: header 'missing_header/missing.h' not found
+// MISSING-HEADER-DAG: auto-import-unavailable.cpp:[[@LINE+1]]:10: note: submodule of top-level module 'missing_header' implicitly imported here
+#include "missing_header/not_missing.h"
+
+// We should not attempt to build the module.
+// MISSING-HEADER-NOT: remark: building module
+
+#endif // #ifdef MISSING_HEADER
+
+
+#ifdef NONREQUIRED_MISSING_HEADER
+
+// However, if the missing header is dominated by an unsatisfied
+// `requires`, then that is acceptable.
+// This also tests that an unsatisfied `requires` elsewhere in the
+// top-level module doesn't affect an available module.
+
+// NONREQUIRED-MISSING-HEADER: auto-import-unavailable.cpp:[[@LINE+2]]:10: remark: building module 'nonrequired_missing_header'
+// NONREQUIRED-MISSING-HEADER: auto-import-unavailable.cpp:[[@LINE+1]]:10: remark: finished building module 'nonrequired_missing_header'
+#include "nonrequired_missing_header/not_missing.h"
+
+#endif // #ifdef NONREQUIRED_MISSING_HEADER
+
+
+#ifdef MISSING_REQUIREMENT
+
+// If the header is unavailable due to a missing requirement, an error
+// should be emitted if a user tries to include it.
+
+// MISSING-REQUIREMENT:module.modulemap:16:8: error: module 'missing_requirement' requires feature 'nonexistent_feature'
+// MISSING-REQUIREMENT: auto-import-unavailable.cpp:[[@LINE+1]]:10: note: submodule of top-level module 'missing_requirement' implicitly imported here
+#include "missing_requirement.h"
+
+// MISSING-REQUIREMENT-NOT: remark: building module
+
+#endif // #ifdef MISSING_REQUIREMENT
Index: test/Modules/Inputs/macro-reexport/module.modulemap
===================================================================
--- test/Modules/Inputs/macro-reexport/module.modulemap
+++ test/Modules/Inputs/macro-reexport/module.modulemap
@@ -19,5 +19,4 @@
}
module f {
module f1 { header "f1.h" export * }
- module f2 { header "f2.h" export * }
}
Index: test/Modules/Inputs/declare-use/module.map
===================================================================
--- test/Modules/Inputs/declare-use/module.map
+++ test/Modules/Inputs/declare-use/module.map
@@ -20,14 +20,12 @@
module XE {
header "e.h"
- header "unavailable.h"
use XA
use XB
}
module XF {
header "f.h"
- header "unavailable.h"
use XA
use XB
}
Index: test/Modules/Inputs/available-is-better/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/available-is-better/module.modulemap
@@ -0,0 +1,17 @@
+// There is some order-dependence to how clang chooses modules, so make
+// sure that both the first and last modules here are ones that would
+// cause a test failure if they were picked.
+
+module unavailable_before {
+ requires nonexistent_feature
+ header "available-is-better.h"
+}
+
+module available {
+ header "available-is-better.h"
+}
+
+module unavailable_after {
+ requires nonexistent_feature
+ header "available-is-better.h"
+}
Index: test/Modules/Inputs/available-is-better/available-is-better.h
===================================================================
--- /dev/null
+++ test/Modules/Inputs/available-is-better/available-is-better.h
@@ -0,0 +1,2 @@
+#pragma once
+int available;
Index: test/Modules/Inputs/auto-import-unavailable/module.modulemap
===================================================================
--- /dev/null
+++ test/Modules/Inputs/auto-import-unavailable/module.modulemap
@@ -0,0 +1,19 @@
+module missing_header {
+ module missing { header "missing_header/missing.h" }
+ module error_importing_this { header "missing_header/not_missing.h" }
+}
+
+module nonrequired_missing_header {
+ module unsatisfied_requires {
+ requires nonexistent_feature
+ header "nonrequired_missing_header/missing.h"
+ }
+ module fine_to_import {
+ header "nonrequired_missing_header/not_missing.h"
+ }
+}
+
+module missing_requirement {
+ requires nonexistent_feature
+ header "missing_requirement.h"
+}
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1660,6 +1660,28 @@
}
}
+ // If this include corresponds to a module but that module is
+ // unavailable, diagnose the situation and bail out.
+ if (SuggestedModule && !SuggestedModule.getModule()->isAvailable()) {
+ clang::Module::Requirement Requirement;
+ clang::Module::UnresolvedHeaderDirective MissingHeader;
+ Module *M = SuggestedModule.getModule();
+ // Identify the cause.
+ (void)M->isAvailable(getLangOpts(), getTargetInfo(), Requirement,
+ MissingHeader);
+ if (MissingHeader.FileNameLoc.isValid()) {
+ Diag(MissingHeader.FileNameLoc, diag::err_module_header_missing)
+ << MissingHeader.IsUmbrella << MissingHeader.FileName;
+ } else {
+ Diag(M->DefinitionLoc, diag::err_module_unavailable)
+ << M->getFullModuleName() << Requirement.second << Requirement.first;
+ }
+ Diag(FilenameTok.getLocation(),
+ diag::note_implicit_top_level_module_import_here)
+ << M->getTopLevelModuleName();
+ return;
+ }
+
// Should we enter the source file? Set to false if either the source file is
// known to have no effect beyond its effect on module visibility -- that is,
// if it's got an include guard that is already defined or is a modular header
Index: lib/Lex/ModuleMap.cpp
===================================================================
--- lib/Lex/ModuleMap.cpp
+++ lib/Lex/ModuleMap.cpp
@@ -320,6 +320,10 @@
static bool isBetterKnownHeader(const ModuleMap::KnownHeader &New,
const ModuleMap::KnownHeader &Old) {
+ // Prefer available modules.
+ if (New.getModule()->isAvailable() && !Old.getModule()->isAvailable())
+ return true;
+
// Prefer a public header over a private header.
if ((New.getRole() & ModuleMap::PrivateHeader) !=
(Old.getRole() & ModuleMap::PrivateHeader))
@@ -349,9 +353,6 @@
// Prefer a header from the current module over all others.
if (H.getModule() == CompilingModule)
return MakeResult(H);
- // Cannot use a module if it is unavailable.
- if (!H.getModule()->isAvailable())
- continue;
if (!Result || isBetterKnownHeader(H, Result))
Result = H;
}
Index: include/clang/Basic/DiagnosticLexKinds.td
===================================================================
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -622,6 +622,8 @@
def warn_auto_module_import : Warning<
"treating #%select{include|import|include_next|__include_macros}0 as an "
"import of module '%1'">, InGroup<AutoImport>, DefaultIgnore;
+def note_implicit_top_level_module_import_here : Note<
+ "submodule of top-level module '%0' implicitly imported here">;
def warn_uncovered_module_header : Warning<
"umbrella header for module '%0' does not include header '%1'">,
InGroup<IncompleteUmbrella>;
Index: include/clang/Basic/DiagnosticFrontendKinds.td
===================================================================
--- include/clang/Basic/DiagnosticFrontendKinds.td
+++ include/clang/Basic/DiagnosticFrontendKinds.td
@@ -173,10 +173,6 @@
"no submodule named %0 in module '%1'; did you mean '%2'?">;
def warn_missing_submodule : Warning<"missing submodule '%0'">,
InGroup<IncompleteUmbrella>;
-def err_module_unavailable : Error<
- "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
-def err_module_header_missing : Error<
- "%select{|umbrella }0header '%1' not found">;
def err_module_cannot_create_includes : Error<
"cannot create includes file for module %0: %1">;
def warn_module_config_macro_undef : Warning<
Index: include/clang/Basic/DiagnosticCommonKinds.td
===================================================================
--- include/clang/Basic/DiagnosticCommonKinds.td
+++ include/clang/Basic/DiagnosticCommonKinds.td
@@ -84,6 +84,10 @@
def err_module_build_disabled: Error<
"module '%0' is needed but has not been provided, and implicit use of module "
"files is disabled">, DefaultFatal;
+def err_module_unavailable : Error<
+ "module '%0' %select{is incompatible with|requires}1 feature '%2'">;
+def err_module_header_missing : Error<
+ "%select{|umbrella }0header '%1' not found">;
def err_module_lock_failure : Error<
"could not acquire lock file for module '%0'">, DefaultFatal;
def err_module_lock_timeout : Error<
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
