From 0761a8a085f404482b375f71e670da8ef2ea612e Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Tue, 17 Dec 2013 10:31:37 +0000 Subject: [PATCH] Modules: Don't warn upon missing headers while reading the module map. Instead, mark the module as unavailable so that clang errors as soon as someone tries to build this module. This works towards the long-term goal of not stat'ing the header files at all while reading the module map and instead read them only when the module is being built (there is a corresponding FIXME in parseHeaderDecl()). However, it seems non-trivial to get there and this unblock us and moves us into the right direction. Also changed the implementation to reuse the same DiagnosticsEngine. llvm-svn: 197485 --- .../clang/Basic/DiagnosticFrontendKinds.td | 2 + .../include/clang/Basic/DiagnosticLexKinds.td | 2 - clang/include/clang/Basic/Module.h | 17 +++++- clang/include/clang/Lex/ModuleMap.h | 4 +- clang/lib/Basic/Module.cpp | 6 +- clang/lib/Frontend/CompilerInstance.cpp | 18 ++++-- clang/lib/Frontend/FrontendActions.cpp | 15 +++-- clang/lib/Lex/HeaderSearch.cpp | 2 +- clang/lib/Lex/ModuleMap.cpp | 55 +++++++++---------- .../test/Modules/Inputs/submodules/module.map | 5 ++ .../unnecessary-module-map-parsing/module.map | 2 +- clang/test/Modules/submodules.cpp | 5 ++ .../Modules/unnecessary-module-map-parsing.c | 2 +- 13 files changed, 86 insertions(+), 49 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index 65a11cbc6655..b0b140d61924 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -140,6 +140,8 @@ def warn_missing_submodule : Warning<"missing submodule '%0'">, InGroup; 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 warn_module_config_macro_undef : Warning< "%select{definition|#undef}0 of configuration macro '%1' has no effect on " "the import of '%2'; pass '%select{-D%1=...|-U%1}0' on the command line " diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index e9de4db03680..e59ded86e79c 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -551,8 +551,6 @@ def err_mmap_expected_mmap_file : Error<"expected a module map file name">; def err_mmap_module_redefinition : Error< "redefinition of module '%0'">; def note_mmap_prev_definition : Note<"previously defined here">; -def err_mmap_header_not_found : Error< - "%select{|umbrella }0header '%1' not found">; def err_mmap_umbrella_dir_not_found : Error< "umbrella directory '%0' not found">; def err_mmap_umbrella_clash : Error< diff --git a/clang/include/clang/Basic/Module.h b/clang/include/clang/Basic/Module.h index e8d774e1eb54..a760ae3ce691 100644 --- a/clang/include/clang/Basic/Module.h +++ b/clang/include/clang/Basic/Module.h @@ -88,7 +88,19 @@ public: SmallVector ExcludedHeaders; /// \brief The headers that are private to this module. - llvm::SmallVector PrivateHeaders; + SmallVector PrivateHeaders; + + /// \brief Information about a header directive as found in the module map + /// file. + struct HeaderDirective { + SourceLocation FileNameLoc; + std::string FileName; + bool IsUmbrella; + }; + + /// \brief Headers that are mentioned in the module map file but could not be + /// found on the file system. + SmallVector MissingHeaders; /// \brief An individual requirement: a feature name and a flag indicating /// the required state of that feature. @@ -276,7 +288,8 @@ public: /// this module. bool isAvailable(const LangOptions &LangOpts, const TargetInfo &Target, - Requirement &Req) const; + Requirement &Req, + HeaderDirective &MissingHeader) const; /// \brief Determine whether this module is a submodule. bool isSubModule() const { return Parent != 0; } diff --git a/clang/include/clang/Lex/ModuleMap.h b/clang/include/clang/Lex/ModuleMap.h index 483ae3d592a0..5339ec13b295 100644 --- a/clang/include/clang/Lex/ModuleMap.h +++ b/clang/include/clang/Lex/ModuleMap.h @@ -38,7 +38,7 @@ class ModuleMapParser; class ModuleMap { SourceManager &SourceMgr; - IntrusiveRefCntPtr Diags; + DiagnosticsEngine &Diags; const LangOptions &LangOpts; const TargetInfo *Target; HeaderSearch &HeaderInfo; @@ -188,7 +188,7 @@ public: /// \param LangOpts Language options for this translation unit. /// /// \param Target The target for this translation unit. - ModuleMap(SourceManager &SourceMgr, DiagnosticConsumer &DC, + ModuleMap(SourceManager &SourceMgr, DiagnosticsEngine &Diags, const LangOptions &LangOpts, const TargetInfo *Target, HeaderSearch &HeaderInfo); diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index d08cef1a1563..c6a1d602a94c 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -69,11 +69,15 @@ static bool hasFeature(StringRef Feature, const LangOptions &LangOpts, bool Module::isAvailable(const LangOptions &LangOpts, const TargetInfo &Target, - Requirement &Req) const { + Requirement &Req, HeaderDirective &MissingHeader) const { if (IsAvailable) return true; for (const Module *Current = this; Current; Current = Current->Parent) { + if (!Current->MissingHeaders.empty()) { + MissingHeader = Current->MissingHeaders.front(); + return false; + } for (unsigned I = 0, N = Current->Requirements.size(); I != N; ++I) { if (hasFeature(Current->Requirements[I].first, LangOpts, Target) != Current->Requirements[I].second) { diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index eccb94cc4a4f..48ba00560710 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -1360,11 +1360,19 @@ CompilerInstance::loadModule(SourceLocation ImportLoc, // Check whether this module is available. clang::Module::Requirement Requirement; - if (!Module->isAvailable(getLangOpts(), getTarget(), Requirement)) { - getDiagnostics().Report(ImportLoc, diag::err_module_unavailable) - << Module->getFullModuleName() - << Requirement.second << Requirement.first - << SourceRange(Path.front().second, Path.back().second); + clang::Module::HeaderDirective 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); + } LastModuleImportLoc = ImportLoc; LastModuleImportResult = ModuleLoadResult(); return ModuleLoadResult(); diff --git a/clang/lib/Frontend/FrontendActions.cpp b/clang/lib/Frontend/FrontendActions.cpp index b5f15830024c..e0c68c84a3e7 100644 --- a/clang/lib/Frontend/FrontendActions.cpp +++ b/clang/lib/Frontend/FrontendActions.cpp @@ -247,10 +247,17 @@ bool GenerateModuleAction::BeginSourceFileAction(CompilerInstance &CI, // Check whether we can build this module at all. clang::Module::Requirement Requirement; - if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement)) { - CI.getDiagnostics().Report(diag::err_module_unavailable) - << Module->getFullModuleName() - << Requirement.second << Requirement.first; + clang::Module::HeaderDirective MissingHeader; + if (!Module->isAvailable(CI.getLangOpts(), CI.getTarget(), Requirement, + MissingHeader)) { + if (MissingHeader.FileNameLoc.isValid()) { + CI.getDiagnostics().Report(diag::err_module_header_missing) + << MissingHeader.IsUmbrella << MissingHeader.FileName; + } else { + CI.getDiagnostics().Report(diag::err_module_unavailable) + << Module->getFullModuleName() + << Requirement.second << Requirement.first; + } return false; } diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp index 5770aaadf404..254c0293df9f 100644 --- a/clang/lib/Lex/HeaderSearch.cpp +++ b/clang/lib/Lex/HeaderSearch.cpp @@ -48,7 +48,7 @@ HeaderSearch::HeaderSearch(IntrusiveRefCntPtr HSOpts, const LangOptions &LangOpts, const TargetInfo *Target) : HSOpts(HSOpts), FileMgr(SourceMgr.getFileManager()), FrameworkMap(64), - ModMap(SourceMgr, *Diags.getClient(), LangOpts, Target, *this) + ModMap(SourceMgr, Diags, LangOpts, Target, *this) { AngledDirIdx = 0; SystemDirIdx = 0; diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index 8a1fcaa69e7a..b9a470312895 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -59,7 +59,7 @@ Module *ModuleMap::resolveModuleId(const ModuleId &Id, Module *Mod, Module *Context = lookupModuleUnqualified(Id[0].first, Mod); if (!Context) { if (Complain) - Diags->Report(Id[0].second, diag::err_mmap_missing_module_unqualified) + Diags.Report(Id[0].second, diag::err_mmap_missing_module_unqualified) << Id[0].first << Mod->getFullModuleName(); return 0; @@ -70,7 +70,7 @@ Module *ModuleMap::resolveModuleId(const ModuleId &Id, Module *Mod, Module *Sub = lookupModuleQualified(Id[I].first, Context); if (!Sub) { if (Complain) - Diags->Report(Id[I].second, diag::err_mmap_missing_module_qualified) + Diags.Report(Id[I].second, diag::err_mmap_missing_module_qualified) << Id[I].first << Context->getFullModuleName() << SourceRange(Id[0].second, Id[I-1].second); @@ -83,19 +83,12 @@ Module *ModuleMap::resolveModuleId(const ModuleId &Id, Module *Mod, return Context; } -ModuleMap::ModuleMap(SourceManager &SourceMgr, DiagnosticConsumer &DC, +ModuleMap::ModuleMap(SourceManager &SourceMgr, DiagnosticsEngine &Diags, const LangOptions &LangOpts, const TargetInfo *Target, HeaderSearch &HeaderInfo) - : SourceMgr(SourceMgr), LangOpts(LangOpts), Target(Target), + : SourceMgr(SourceMgr), Diags(Diags), LangOpts(LangOpts), Target(Target), HeaderInfo(HeaderInfo), BuiltinIncludeDir(0), CompilingModule(0), - SourceModule(0) { - IntrusiveRefCntPtr DiagIDs(new DiagnosticIDs); - Diags = IntrusiveRefCntPtr( - new DiagnosticsEngine(DiagIDs, new DiagnosticOptions)); - Diags->setClient(new ForwardingDiagnosticConsumer(DC), - /*ShouldOwnClient=*/true); - Diags->setSourceManager(&SourceMgr); -} + SourceModule(0) {} ModuleMap::~ModuleMap() { for (llvm::StringMap::iterator I = Modules.begin(), @@ -1480,12 +1473,13 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, HadError = true; return; } - std::string FileName = Tok.getString(); - SourceLocation FileNameLoc = consumeToken(); + Module::HeaderDirective Header; + Header.FileName = Tok.getString(); + Header.FileNameLoc = consumeToken(); // Check whether we already have an umbrella. if (LeadingToken == MMToken::UmbrellaKeyword && ActiveModule->Umbrella) { - Diags.Report(FileNameLoc, diag::err_mmap_umbrella_clash) + Diags.Report(Header.FileNameLoc, diag::err_mmap_umbrella_clash) << ActiveModule->getFullModuleName(); HadError = true; return; @@ -1495,12 +1489,12 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, const FileEntry *File = 0; const FileEntry *BuiltinFile = 0; SmallString<128> PathName; - if (llvm::sys::path::is_absolute(FileName)) { - PathName = FileName; + if (llvm::sys::path::is_absolute(Header.FileName)) { + PathName = Header.FileName; File = SourceMgr.getFileManager().getFile(PathName); } else if (const DirectoryEntry *Dir = getOverriddenHeaderSearchDir()) { PathName = Dir->getName(); - llvm::sys::path::append(PathName, FileName); + llvm::sys::path::append(PathName, Header.FileName); File = SourceMgr.getFileManager().getFile(PathName); } else { // Search for the header file within the search directory. @@ -1511,18 +1505,18 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, appendSubframeworkPaths(ActiveModule, PathName); // Check whether this file is in the public headers. - llvm::sys::path::append(PathName, "Headers", FileName); + llvm::sys::path::append(PathName, "Headers", Header.FileName); File = SourceMgr.getFileManager().getFile(PathName); if (!File) { // Check whether this file is in the private headers. PathName.resize(PathLength); - llvm::sys::path::append(PathName, "PrivateHeaders", FileName); + llvm::sys::path::append(PathName, "PrivateHeaders", Header.FileName); File = SourceMgr.getFileManager().getFile(PathName); } } else { // Lookup for normal headers. - llvm::sys::path::append(PathName, FileName); + llvm::sys::path::append(PathName, Header.FileName); File = SourceMgr.getFileManager().getFile(PathName); // If this is a system module with a top-level header, this header @@ -1530,9 +1524,9 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, // supplied by Clang. Find that builtin header. if (ActiveModule->IsSystem && LeadingToken != MMToken::UmbrellaKeyword && BuiltinIncludeDir && BuiltinIncludeDir != Directory && - isBuiltinHeader(FileName)) { + isBuiltinHeader(Header.FileName)) { SmallString<128> BuiltinPathName(BuiltinIncludeDir->getName()); - llvm::sys::path::append(BuiltinPathName, FileName); + llvm::sys::path::append(BuiltinPathName, Header.FileName); BuiltinFile = SourceMgr.getFileManager().getFile(BuiltinPathName); // If Clang supplies this header but the underlying system does not, @@ -1577,10 +1571,13 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, } } else if (LeadingToken != MMToken::ExcludeKeyword) { // Ignore excluded header files. They're optional anyway. - - Diags.Report(FileNameLoc, diag::err_mmap_header_not_found) - << (LeadingToken == MMToken::UmbrellaKeyword) << FileName; - HadError = true; + + // If we find a module that has a missing header, we mark this module as + // unavailable and store the header directive for displaying diagnostics. + // Other submodules in the same module can still be used. + Header.IsUmbrella = LeadingToken == MMToken::UmbrellaKeyword; + ActiveModule->IsAvailable = false; + ActiveModule->MissingHeaders.push_back(Header); } } @@ -2119,11 +2116,9 @@ bool ModuleMap::parseModuleMapFile(const FileEntry *File, bool IsSystem) { // Parse this module map file. Lexer L(ID, SourceMgr.getBuffer(ID), SourceMgr, MMapLangOpts); - Diags->getClient()->BeginSourceFile(MMapLangOpts); - ModuleMapParser Parser(L, SourceMgr, Target, *Diags, *this, File->getDir(), + ModuleMapParser Parser(L, SourceMgr, Target, Diags, *this, File->getDir(), BuiltinIncludeDir, IsSystem); bool Result = Parser.parseModuleMapFile(); - Diags->getClient()->EndSourceFile(); ParsedModuleMap[File] = Result; return Result; } diff --git a/clang/test/Modules/Inputs/submodules/module.map b/clang/test/Modules/Inputs/submodules/module.map index c91e94f47d23..15931031abba 100644 --- a/clang/test/Modules/Inputs/submodules/module.map +++ b/clang/test/Modules/Inputs/submodules/module.map @@ -10,3 +10,8 @@ module import_self { module c { header "import-self-c.h" } module d { header "import-self-d.h" } } + +module missing_headers { + module missing { header "missing.h" } + module not_missing { header "not_missing.h" } +} diff --git a/clang/test/Modules/Inputs/unnecessary-module-map-parsing/module.map b/clang/test/Modules/Inputs/unnecessary-module-map-parsing/module.map index 2d2104177ff5..6d4ceeeb0cca 100644 --- a/clang/test/Modules/Inputs/unnecessary-module-map-parsing/module.map +++ b/clang/test/Modules/Inputs/unnecessary-module-map-parsing/module.map @@ -1,3 +1,3 @@ module a { - header "unknown.h" + eader "unknown.h" } diff --git a/clang/test/Modules/submodules.cpp b/clang/test/Modules/submodules.cpp index 9c62389eadc0..12bf87f6307e 100644 --- a/clang/test/Modules/submodules.cpp +++ b/clang/test/Modules/submodules.cpp @@ -34,3 +34,8 @@ extern MyTypeC import_self_test_c; // FIXME: This should be valid; import_self.b re-exports import_self.d. extern MyTypeD import_self_test_d; // expected-error {{must be imported from module 'import_self.d'}} // expected-note@import-self-d.h:1 {{here}} + +// expected-error@Inputs/submodules/module.map:15{{header 'missing.h' not found}} +@import missing_headers.missing; +@import missing_headers.not_missing; +void f() { NotMissingFunction(); }; diff --git a/clang/test/Modules/unnecessary-module-map-parsing.c b/clang/test/Modules/unnecessary-module-map-parsing.c index 16f7086a6d7a..4c83448179e0 100644 --- a/clang/test/Modules/unnecessary-module-map-parsing.c +++ b/clang/test/Modules/unnecessary-module-map-parsing.c @@ -3,6 +3,6 @@ // RUN: not %clang_cc1 -fmodules -I %S/Inputs/unnecessary-module-map-parsing -fsyntax-only %s 2>&1 | FileCheck %s // RUN: %clang_cc1 -I %S/Inputs/unnecessary-module-map-parsing -fsyntax-only %s -// CHECK: error: header 'unknown.h' not found +// CHECK: error: expected umbrella, header, submodule, or module export #include "a1.h"