r245228 - [modules] PR20507: Avoid silent textual inclusion.
Sean Silva via cfe-commits
cfe-commits at lists.llvm.org
Mon Aug 17 09:39:34 PDT 2015
Author: silvas
Date: Mon Aug 17 11:39:30 2015
New Revision: 245228
URL: http://llvm.org/viewvc/llvm-project?rev=245228&view=rev
Log:
[modules] PR20507: Avoid silent textual inclusion.
Summary:
If a module was unavailable (either a missing requirement on the module
being imported, or a missing file anywhere in the top-level module (and
not dominated by an unsatisfied `requires`)), we would silently treat
inclusions as textual. This would cause all manner of crazy and
confusing errors (and would also silently "work" sometimes, making the
problem difficult to track down).
I'm really not a fan of the `M->isAvailable(getLangOpts(), getTargetInfo(),
Requirement, MissingHeader)` function; it seems to do too many things at
once, but for now I've done things in a sort of awkward way.
The changes to test/Modules/Inputs/declare-use/module.map
were necessitated because the thing that was meant to be tested there
(introduced in r197805) was predicated on silently falling back to textual
inclusion, which we no longer do.
The changes to test/Modules/Inputs/macro-reexport/module.modulemap
are just an overlooked missing header that seems to have been missing since
this code was committed (r213922), which is now caught.
Reviewers: rsmith, benlangmuir, djasper
Subscribers: cfe-commits
Differential Revision: http://reviews.llvm.org/D10423
Added:
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_requirement.h
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/module.modulemap
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h
cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h
cfe/trunk/test/Modules/Inputs/available-is-better/
cfe/trunk/test/Modules/Inputs/available-is-better/available-is-better.h
cfe/trunk/test/Modules/Inputs/available-is-better/module.modulemap
cfe/trunk/test/Modules/auto-import-unavailable.cpp
cfe/trunk/test/Modules/available-is-better.cpp
Modified:
cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/ModuleMap.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp
cfe/trunk/test/Modules/Inputs/declare-use/module.map
cfe/trunk/test/Modules/Inputs/macro-reexport/module.modulemap
Modified: cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticCommonKinds.td Mon Aug 17 11:39:30 2015
@@ -84,6 +84,10 @@ def err_module_not_built : Error<"could
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<
Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Aug 17 11:39:30 2015
@@ -183,10 +183,6 @@ 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<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<
Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Mon Aug 17 11:39:30 2015
@@ -623,6 +623,8 @@ def warn_mmap_unknown_attribute : Warnin
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>;
Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Mon Aug 17 11:39:30 2015
@@ -320,6 +320,10 @@ void ModuleMap::diagnoseHeaderInclusion(
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 @@ ModuleMap::KnownHeader ModuleMap::findMo
// Prefer a header from the current module over all others.
if (H.getModule()->getTopLevelModule() == CompilingModule)
return MakeResult(H);
- // Cannot use a module if it is unavailable.
- if (!H.getModule()->isAvailable())
- continue;
if (!Result || isBetterKnownHeader(H, Result))
Result = H;
}
Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Aug 17 11:39:30 2015
@@ -1674,6 +1674,29 @@ void Preprocessor::HandleIncludeDirectiv
getLangOpts().CurrentModule &&
SuggestedModule.getModule()->getTopLevelModuleName() !=
getLangOpts().ImplementationOfModule) {
+
+ // If this include corresponds to a module but that module is
+ // unavailable, diagnose the situation and bail out.
+ if (!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;
+ }
+
// Compute the module access path corresponding to this module.
// FIXME: Should we have a second loadModule() overload to avoid this
// extra lookup step?
Added: cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h (added)
+++ cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_header/not_missing.h Mon Aug 17 11:39:30 2015
@@ -0,0 +1 @@
+// missing_header/not_missing.h
Added: cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_requirement.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_requirement.h?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_requirement.h (added)
+++ cfe/trunk/test/Modules/Inputs/auto-import-unavailable/missing_requirement.h Mon Aug 17 11:39:30 2015
@@ -0,0 +1 @@
+// missing_requirement.h
Added: cfe/trunk/test/Modules/Inputs/auto-import-unavailable/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/auto-import-unavailable/module.modulemap?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/auto-import-unavailable/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/auto-import-unavailable/module.modulemap Mon Aug 17 11:39:30 2015
@@ -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"
+}
Added: cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h (added)
+++ cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/not_missing.h Mon Aug 17 11:39:30 2015
@@ -0,0 +1 @@
+// nonrequired_missing_header/not_missing.h
Added: cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h (added)
+++ cfe/trunk/test/Modules/Inputs/auto-import-unavailable/nonrequired_missing_header/requires_feature_you_dont_have.h Mon Aug 17 11:39:30 2015
@@ -0,0 +1 @@
+// nonrequired_missing_header/requires_feature_you_dont_have.h
Added: cfe/trunk/test/Modules/Inputs/available-is-better/available-is-better.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/available-is-better/available-is-better.h?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/available-is-better/available-is-better.h (added)
+++ cfe/trunk/test/Modules/Inputs/available-is-better/available-is-better.h Mon Aug 17 11:39:30 2015
@@ -0,0 +1,2 @@
+#pragma once
+int available;
Added: cfe/trunk/test/Modules/Inputs/available-is-better/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/available-is-better/module.modulemap?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/available-is-better/module.modulemap (added)
+++ cfe/trunk/test/Modules/Inputs/available-is-better/module.modulemap Mon Aug 17 11:39:30 2015
@@ -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"
+}
Modified: cfe/trunk/test/Modules/Inputs/declare-use/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/declare-use/module.map?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/declare-use/module.map (original)
+++ cfe/trunk/test/Modules/Inputs/declare-use/module.map Mon Aug 17 11:39:30 2015
@@ -20,14 +20,12 @@ module XD {
module XE {
header "e.h"
- header "unavailable.h"
use XA
use XB
}
module XF {
header "f.h"
- header "unavailable.h"
use XA
use XB
}
Modified: cfe/trunk/test/Modules/Inputs/macro-reexport/module.modulemap
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/macro-reexport/module.modulemap?rev=245228&r1=245227&r2=245228&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/macro-reexport/module.modulemap (original)
+++ cfe/trunk/test/Modules/Inputs/macro-reexport/module.modulemap Mon Aug 17 11:39:30 2015
@@ -19,5 +19,4 @@ module e {
}
module f {
module f1 { header "f1.h" export * }
- module f2 { header "f2.h" export * }
}
Added: cfe/trunk/test/Modules/auto-import-unavailable.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/auto-import-unavailable.cpp?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/auto-import-unavailable.cpp (added)
+++ cfe/trunk/test/Modules/auto-import-unavailable.cpp Mon Aug 17 11:39:30 2015
@@ -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
Added: cfe/trunk/test/Modules/available-is-better.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/available-is-better.cpp?rev=245228&view=auto
==============================================================================
--- cfe/trunk/test/Modules/available-is-better.cpp (added)
+++ cfe/trunk/test/Modules/available-is-better.cpp Mon Aug 17 11:39:30 2015
@@ -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
More information about the cfe-commits
mailing list