From: Richard Smith Date: Mon, 5 Jun 2017 18:57:56 +0000 (+0000) Subject: Factor out and unify emission of "module is unavailable" diagnostics. X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=747fcbb8908f48825386c6c9eb8fb2fd4bf9444d;p=clang Factor out and unify emission of "module is unavailable" diagnostics. Inspired by post-commit review of r304190. git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@304728 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/include/clang/Basic/DiagnosticFrontendKinds.td b/include/clang/Basic/DiagnosticFrontendKinds.td index 1267f8d09f..8b4cb47e54 100644 --- a/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/include/clang/Basic/DiagnosticFrontendKinds.td @@ -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; +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< diff --git a/include/clang/Lex/Preprocessor.h b/include/clang/Lex/Preprocessor.h index aeca83a907..7958cd0e1a 100644 --- a/include/clang/Lex/Preprocessor.h +++ b/include/clang/Lex/Preprocessor.h @@ -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 diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index e92672a785..3e390c96e1 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -1824,20 +1824,10 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, } // 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(); diff --git a/lib/Frontend/FrontendAction.cpp b/lib/Frontend/FrontendAction.cpp index ef98172b55..2de8f06e71 100644 --- a/lib/Frontend/FrontendAction.cpp +++ b/lib/Frontend/FrontendAction.cpp @@ -443,21 +443,9 @@ static Module *prepareToBuildModule(CompilerInstance &CI, } // 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. diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp index 2d3ad69098..b2c3c2e707 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -1654,6 +1654,26 @@ static bool trySimplifyPath(SmallVectorImpl &Components, 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::HandleIncludeDirective(SourceLocation HashLoc, // 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; } diff --git a/lib/Lex/Pragma.cpp b/lib/Lex/Pragma.cpp index e1d981527b..dd52abd74f 100644 --- a/lib/Lex/Pragma.cpp +++ b/lib/Lex/Pragma.cpp @@ -1408,18 +1408,8 @@ struct PragmaModuleBeginHandler : public PragmaHandler { } // 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; diff --git a/test/Modules/requires-coroutines.mm b/test/Modules/requires-coroutines.mm index d3519cd2bb..8e25a3c575 100644 --- a/test/Modules/requires-coroutines.mm +++ b/test/Modules/requires-coroutines.mm @@ -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-error@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-error@module.map:25 {{module 'DependsOnModule.Coroutines' requires feature 'coroutines'}} +@import DependsOnModule.Coroutines; // expected-note {{module imported here}} #endif diff --git a/test/Modules/requires-gnuinlineasm.m b/test/Modules/requires-gnuinlineasm.m index 80b1b18d07..e710b6bf8d 100644 --- a/test/Modules/requires-gnuinlineasm.m +++ b/test/Modules/requires-gnuinlineasm.m @@ -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 diff --git a/test/Modules/requires.m b/test/Modules/requires.m index 1a01372710..d61de6bd48 100644 --- a/test/Modules/requires.m +++ b/test/Modules/requires.m @@ -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-error@module.map:* {{module 'RequiresWithMissingHeader.HeaderBefore' requires feature 'missing'}} +@import RequiresWithMissingHeader.HeaderBefore; // expected-note {{module imported here}} +// expected-error@module.map:* {{module 'RequiresWithMissingHeader.HeaderAfter' requires feature 'missing'}} +@import RequiresWithMissingHeader.HeaderAfter; // expected-note {{module imported here}} diff --git a/test/Modules/requires.mm b/test/Modules/requires.mm index cc64a500bd..f90622ece8 100644 --- a/test/Modules/requires.mm +++ b/test/Modules/requires.mm @@ -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-error@module.map:11 {{module 'DependsOnModule.NotCXX' is incompatible with feature 'cplusplus'}} +@import DependsOnModule.NotCXX; // expected-note {{module imported here}} +// expected-error@module.map:15 {{module 'DependsOnModule.NotObjC' is incompatible with feature 'objc'}} +@import DependsOnModule.NotObjC; // expected-note {{module imported here}}