Hi rsmith, benlangmuir, djasper, This pretty naively follows the implementation suggested by Richard.
I'd like to switch these over to be fatal errors (since they are essentially similar to err_pp_file_not_found), but I'm still investigating some test failures that happen when I switched them over. 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. http://reviews.llvm.org/D10423 Files: include/clang/Basic/DiagnosticCommonKinds.td include/clang/Basic/DiagnosticFrontendKinds.td lib/Lex/ModuleMap.cpp lib/Lex/PPDirectives.cpp test/Modules/Inputs/declare-use/module.map test/Modules/Inputs/macro-reexport/module.modulemap test/Modules/missing-header.cpp EMAIL PREFERENCES http://reviews.llvm.org/settings/panel/emailpreferences/
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< 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: lib/Lex/ModuleMap.cpp =================================================================== --- lib/Lex/ModuleMap.cpp +++ lib/Lex/ModuleMap.cpp @@ -341,13 +341,9 @@ if (Known != Headers.end()) { ModuleMap::KnownHeader Result; // Iterate over all modules that 'File' is part of to find the best fit. - for (KnownHeader &H : Known->second) { - // Cannot use a module if it is unavailable. - if (!H.getModule()->isAvailable()) - continue; + for (KnownHeader &H : Known->second) if (!Result || isBetterKnownHeader(H, Result)) Result = H; - } return MakeResult(Result); } Index: lib/Lex/PPDirectives.cpp =================================================================== --- lib/Lex/PPDirectives.cpp +++ lib/Lex/PPDirectives.cpp @@ -1654,6 +1654,25 @@ } } + // 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(MissingHeader.FileNameLoc, diag::err_module_unavailable) + << M->getFullModuleName() << Requirement.second << Requirement.first; + } + 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: 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/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/missing-header.cpp =================================================================== --- /dev/null +++ test/Modules/missing-header.cpp @@ -0,0 +1,12 @@ +// RUN: rm -rf %t +// RUN: not %clang_cc1 -x c++ -fmodules -fmodules-cache-path=%t -I %S/Inputs/submodules %s 2>&1 | FileCheck %s + +// 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. + +#include "not_missing.h" + +// CHECK: module.map:15:27: error: header 'missing.h' not found +// We should not attempt to build the module. +// CHECK-NOT: could not build module 'missing_headers'
_______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits