Author: rsmith Date: Mon Jun 5 13:57:56 2017 New Revision: 304728 URL: http://llvm.org/viewvc/llvm-project?rev=304728&view=rev Log: Factor out and unify emission of "module is unavailable" diagnostics.
Inspired by post-commit review of r304190. Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td cfe/trunk/include/clang/Lex/Preprocessor.h cfe/trunk/lib/Frontend/CompilerInstance.cpp cfe/trunk/lib/Frontend/FrontendAction.cpp cfe/trunk/lib/Lex/PPDirectives.cpp cfe/trunk/lib/Lex/Pragma.cpp cfe/trunk/test/Modules/requires-coroutines.mm cfe/trunk/test/Modules/requires-gnuinlineasm.m cfe/trunk/test/Modules/requires.m cfe/trunk/test/Modules/requires.mm Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original) +++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Jun 5 13:57:56 2017 @@ -196,6 +196,7 @@ 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 note_module_import_here : Note<"module imported here">; 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/Lex/Preprocessor.h URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/Preprocessor.h?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/include/clang/Lex/Preprocessor.h (original) +++ cfe/trunk/include/clang/Lex/Preprocessor.h Mon Jun 5 13:57:56 2017 @@ -1954,6 +1954,13 @@ private: void HandleMicrosoftImportDirective(Token &Tok); public: + /// Check that the given module is available, producing a diagnostic if not. + /// \return \c true if the check failed (because the module is not available). + /// \c false if the module appears to be usable. + static bool checkModuleIsAvailable(const LangOptions &LangOpts, + const TargetInfo &TargetInfo, + DiagnosticsEngine &Diags, Module *M); + // Module inclusion testing. /// \brief Find the module that owns the source or header file that /// \p Loc points to. If the location is in a file that was included Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original) +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Jun 5 13:57:56 2017 @@ -1824,20 +1824,10 @@ CompilerInstance::loadModule(SourceLocat } // Check whether this module is available. - clang::Module::Requirement Requirement; - clang::Module::UnresolvedHeaderDirective MissingHeader; - if (!Module->isAvailable(getLangOpts(), getTarget(), Requirement, - MissingHeader)) { - if (MissingHeader.FileNameLoc.isValid()) { - getDiagnostics().Report(MissingHeader.FileNameLoc, - diag::err_module_header_missing) - << MissingHeader.IsUmbrella << MissingHeader.FileName; - } else { - getDiagnostics().Report(ImportLoc, diag::err_module_unavailable) - << Module->getFullModuleName() - << Requirement.second << Requirement.first - << SourceRange(Path.front().second, Path.back().second); - } + if (Preprocessor::checkModuleIsAvailable(getLangOpts(), getTarget(), + getDiagnostics(), Module)) { + getDiagnostics().Report(ImportLoc, diag::note_module_import_here) + << SourceRange(Path.front().second, Path.back().second); LastModuleImportLoc = ImportLoc; LastModuleImportResult = ModuleLoadResult(); return ModuleLoadResult(); Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/lib/Frontend/FrontendAction.cpp (original) +++ cfe/trunk/lib/Frontend/FrontendAction.cpp Mon Jun 5 13:57:56 2017 @@ -443,21 +443,9 @@ static Module *prepareToBuildModule(Comp } // Check whether we can build this module at all. - clang::Module::Requirement Requirement; - clang::Module::UnresolvedHeaderDirective MissingHeader; - if (!M->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement, - MissingHeader)) { - if (MissingHeader.FileNameLoc.isValid()) { - CI.getDiagnostics().Report(MissingHeader.FileNameLoc, - diag::err_module_header_missing) - << MissingHeader.IsUmbrella << MissingHeader.FileName; - } else { - CI.getDiagnostics().Report(diag::err_module_unavailable) - << M->getFullModuleName() << Requirement.second << Requirement.first; - } - + if (Preprocessor::checkModuleIsAvailable(CI.getLangOpts(), CI.getTarget(), + CI.getDiagnostics(), M)) return nullptr; - } // Inform the preprocessor that includes from within the input buffer should // be resolved relative to the build directory of the module map file. Modified: cfe/trunk/lib/Lex/PPDirectives.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/lib/Lex/PPDirectives.cpp (original) +++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Jun 5 13:57:56 2017 @@ -1654,6 +1654,26 @@ static bool trySimplifyPath(SmallVectorI return SuggestReplacement; } +bool Preprocessor::checkModuleIsAvailable(const LangOptions &LangOpts, + const TargetInfo &TargetInfo, + DiagnosticsEngine &Diags, Module *M) { + Module::Requirement Requirement; + Module::UnresolvedHeaderDirective MissingHeader; + if (M->isAvailable(LangOpts, TargetInfo, Requirement, MissingHeader)) + return false; + + if (MissingHeader.FileNameLoc.isValid()) { + Diags.Report(MissingHeader.FileNameLoc, diag::err_module_header_missing) + << MissingHeader.IsUmbrella << MissingHeader.FileName; + } else { + // FIXME: Track the location at which the requirement was specified, and + // use it here. + Diags.Report(M->DefinitionLoc, diag::err_module_unavailable) + << M->getFullModuleName() << Requirement.second << Requirement.first; + } + return true; +} + /// HandleIncludeDirective - The "\#include" tokens have just been read, read /// the file to be included from the lexer, then include it! This is a common /// routine with functionality shared between \#include, \#include_next and @@ -1835,23 +1855,11 @@ void Preprocessor::HandleIncludeDirectiv // unavailable, diagnose the situation and bail out. // FIXME: Remove this; loadModule does the same check (but produces // slightly worse diagnostics). - if (!SuggestedModule.getModule()->isAvailable()) { - Module::Requirement Requirement; - 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; - } + if (checkModuleIsAvailable(getLangOpts(), getTargetInfo(), getDiagnostics(), + SuggestedModule.getModule())) { Diag(FilenameTok.getLocation(), diag::note_implicit_top_level_module_import_here) - << M->getTopLevelModuleName(); + << SuggestedModule.getModule()->getTopLevelModuleName(); return; } Modified: cfe/trunk/lib/Lex/Pragma.cpp URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/Pragma.cpp?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/lib/Lex/Pragma.cpp (original) +++ cfe/trunk/lib/Lex/Pragma.cpp Mon Jun 5 13:57:56 2017 @@ -1408,18 +1408,8 @@ struct PragmaModuleBeginHandler : public } // If the module isn't available, it doesn't make sense to enter it. - if (!M->isAvailable()) { - Module::Requirement Requirement; - Module::UnresolvedHeaderDirective MissingHeader; - (void)M->isAvailable(PP.getLangOpts(), PP.getTargetInfo(), - Requirement, MissingHeader); - if (MissingHeader.FileNameLoc.isValid()) { - PP.Diag(MissingHeader.FileNameLoc, diag::err_module_header_missing) - << MissingHeader.IsUmbrella << MissingHeader.FileName; - } else { - PP.Diag(M->DefinitionLoc, diag::err_module_unavailable) - << M->getFullModuleName() << Requirement.second << Requirement.first; - } + if (Preprocessor::checkModuleIsAvailable( + PP.getLangOpts(), PP.getTargetInfo(), PP.getDiagnostics(), M)) { PP.Diag(BeginLoc, diag::note_pp_module_begin_here) << M->getTopLevelModuleName(); return; Modified: cfe/trunk/test/Modules/requires-coroutines.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/requires-coroutines.mm?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/test/Modules/requires-coroutines.mm (original) +++ cfe/trunk/test/Modules/requires-coroutines.mm Mon Jun 5 13:57:56 2017 @@ -1,12 +1,13 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -fcoroutines-ts -DCOROUTINES - +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %s -verify -fcoroutines-ts -DCOROUTINES #ifdef COROUTINES @import DependsOnModule.Coroutines; -@import DependsOnModule.NotCoroutines; // expected-error {{module 'DependsOnModule.NotCoroutines' is incompatible with feature 'coroutines'}} +// expected-er...@module.map:29 {{module 'DependsOnModule.NotCoroutines' is incompatible with feature 'coroutines'}} +@import DependsOnModule.NotCoroutines; // expected-note {{module imported here}} #else @import DependsOnModule.NotCoroutines; -@import DependsOnModule.Coroutines; // expected-error {{module 'DependsOnModule.Coroutines' requires feature 'coroutines'}} +// expected-er...@module.map:25 {{module 'DependsOnModule.Coroutines' requires feature 'coroutines'}} +@import DependsOnModule.Coroutines; // expected-note {{module imported here}} #endif Modified: cfe/trunk/test/Modules/requires-gnuinlineasm.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/requires-gnuinlineasm.m?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/test/Modules/requires-gnuinlineasm.m (original) +++ cfe/trunk/test/Modules/requires-gnuinlineasm.m Mon Jun 5 13:57:56 2017 @@ -1,6 +1,7 @@ // RUN: rm -rf %t // RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules \ // RUN: -fimplicit-module-maps -F %S/Inputs/GNUAsm %s \ +// RUN: -I %S/Inputs/GNUAsm \ // RUN: -fno-gnu-inline-asm -DNO_ASM_INLINE -verify // RUN: rm -rf %t // RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules \ @@ -8,7 +9,8 @@ // RUN: -DASM_INLINE -verify #ifdef NO_ASM_INLINE -@import NeedsGNUInlineAsm.Asm; // expected-error{{module 'NeedsGNUInlineAsm.Asm' requires feature 'gnuinlineasm'}} +// expected-error@NeedsGNUInlineAsm.framework/module.map:4 {{module 'NeedsGNUInlineAsm.Asm' requires feature 'gnuinlineasm'}} +@import NeedsGNUInlineAsm.Asm; // expected-note {{module imported here}} #endif #ifdef ASM_INLINE Modified: cfe/trunk/test/Modules/requires.m URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/requires.m?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/test/Modules/requires.m (original) +++ cfe/trunk/test/Modules/requires.m Mon Jun 5 13:57:56 2017 @@ -1,12 +1,17 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify -fmodule-feature custom_req1 +// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs %s -verify -fmodule-feature custom_req1 -@import DependsOnModule.CXX; // expected-error{{module 'DependsOnModule.CXX' requires feature 'cplusplus'}} +// expected-error@DependsOnModule.framework/module.map:7 {{module 'DependsOnModule.CXX' requires feature 'cplusplus'}} +@import DependsOnModule.CXX; // expected-note {{module imported here}} @import DependsOnModule.NotCXX; -@import DependsOnModule.NotObjC; // expected-error{{module 'DependsOnModule.NotObjC' is incompatible with feature 'objc'}} +// expected-error@DependsOnModule.framework/module.map:15 {{module 'DependsOnModule.NotObjC' is incompatible with feature 'objc'}} +@import DependsOnModule.NotObjC; // expected-note {{module imported here}} @import DependsOnModule.CustomReq1; // OK -@import DependsOnModule.CustomReq2; // expected-error{{module 'DependsOnModule.CustomReq2' requires feature 'custom_req2'}} +// expected-error@DependsOnModule.framework/module.map:22 {{module 'DependsOnModule.CustomReq2' requires feature 'custom_req2'}} +@import DependsOnModule.CustomReq2; // expected-note {{module imported here}} @import RequiresWithMissingHeader; // OK -@import RequiresWithMissingHeader.HeaderBefore; // expected-error{{module 'RequiresWithMissingHeader.HeaderBefore' requires feature 'missing'}} -@import RequiresWithMissingHeader.HeaderAfter; // expected-error{{module 'RequiresWithMissingHeader.HeaderAfter' requires feature 'missing'}} +// expected-er...@module.map:* {{module 'RequiresWithMissingHeader.HeaderBefore' requires feature 'missing'}} +@import RequiresWithMissingHeader.HeaderBefore; // expected-note {{module imported here}} +// expected-er...@module.map:* {{module 'RequiresWithMissingHeader.HeaderAfter' requires feature 'missing'}} +@import RequiresWithMissingHeader.HeaderAfter; // expected-note {{module imported here}} Modified: cfe/trunk/test/Modules/requires.mm URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/requires.mm?rev=304728&r1=304727&r2=304728&view=diff ============================================================================== --- cfe/trunk/test/Modules/requires.mm (original) +++ cfe/trunk/test/Modules/requires.mm Mon Jun 5 13:57:56 2017 @@ -1,6 +1,8 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs %s -verify +// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -F %S/Inputs -I %S/Inputs/DependsOnModule.framework %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'}} +// expected-er...@module.map:11 {{module 'DependsOnModule.NotCXX' is incompatible with feature 'cplusplus'}} +@import DependsOnModule.NotCXX; // expected-note {{module imported here}} +// expected-er...@module.map:15 {{module 'DependsOnModule.NotObjC' is incompatible with feature 'objc'}} +@import DependsOnModule.NotObjC; // expected-note {{module imported here}} _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits