r197485 - Modules: Don't warn upon missing headers while reading the module map.

Daniel Jasper djasper at google.com
Tue Dec 17 02:31:38 PST 2013


Author: djasper
Date: Tue Dec 17 04:31:37 2013
New Revision: 197485

URL: http://llvm.org/viewvc/llvm-project?rev=197485&view=rev
Log:
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.

Modified:
    cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
    cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
    cfe/trunk/include/clang/Basic/Module.h
    cfe/trunk/include/clang/Lex/ModuleMap.h
    cfe/trunk/lib/Basic/Module.cpp
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/lib/Frontend/FrontendActions.cpp
    cfe/trunk/lib/Lex/HeaderSearch.cpp
    cfe/trunk/lib/Lex/ModuleMap.cpp
    cfe/trunk/test/Modules/Inputs/submodules/module.map
    cfe/trunk/test/Modules/Inputs/unnecessary-module-map-parsing/module.map
    cfe/trunk/test/Modules/submodules.cpp
    cfe/trunk/test/Modules/unnecessary-module-map-parsing.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Tue Dec 17 04:31:37 2013
@@ -140,6 +140,8 @@ def warn_missing_submodule : Warning<"mi
   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 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 "

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Tue Dec 17 04:31:37 2013
@@ -551,8 +551,6 @@ def err_mmap_expected_mmap_file : Error<
 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<

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Tue Dec 17 04:31:37 2013
@@ -88,7 +88,19 @@ public:
   SmallVector<const FileEntry *, 2> ExcludedHeaders;
 
   /// \brief The headers that are private to this module.
-  llvm::SmallVector<const FileEntry *, 2> PrivateHeaders;
+  SmallVector<const FileEntry *, 2> 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<HeaderDirective, 1> 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; }

Modified: cfe/trunk/include/clang/Lex/ModuleMap.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleMap.h?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/include/clang/Lex/ModuleMap.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleMap.h Tue Dec 17 04:31:37 2013
@@ -38,7 +38,7 @@ class ModuleMapParser;
   
 class ModuleMap {
   SourceManager &SourceMgr;
-  IntrusiveRefCntPtr<DiagnosticsEngine> 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);
 

Modified: cfe/trunk/lib/Basic/Module.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/Module.cpp (original)
+++ cfe/trunk/lib/Basic/Module.cpp Tue Dec 17 04:31:37 2013
@@ -69,11 +69,15 @@ static bool hasFeature(StringRef Feature
 
 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) {

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Tue Dec 17 04:31:37 2013
@@ -1360,11 +1360,19 @@ CompilerInstance::loadModule(SourceLocat
 
     // 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();

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Tue Dec 17 04:31:37 2013
@@ -247,10 +247,17 @@ bool GenerateModuleAction::BeginSourceFi
 
   // 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;
   }

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Tue Dec 17 04:31:37 2013
@@ -48,7 +48,7 @@ HeaderSearch::HeaderSearch(IntrusiveRefC
                            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;

Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
+++ cfe/trunk/lib/Lex/ModuleMap.cpp Tue Dec 17 04:31:37 2013
@@ -59,7 +59,7 @@ Module *ModuleMap::resolveModuleId(const
   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
     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
   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<DiagnosticIDs> DiagIDs(new DiagnosticIDs);
-  Diags = IntrusiveRefCntPtr<DiagnosticsEngine>(
-            new DiagnosticsEngine(DiagIDs, new DiagnosticOptions));
-  Diags->setClient(new ForwardingDiagnosticConsumer(DC),
-                   /*ShouldOwnClient=*/true);
-  Diags->setSourceManager(&SourceMgr);
-}
+      SourceModule(0) {}
 
 ModuleMap::~ModuleMap() {
   for (llvm::StringMap<Module *>::iterator I = Modules.begin(), 
@@ -1480,12 +1473,13 @@ void ModuleMapParser::parseHeaderDecl(MM
     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(MM
   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(MM
       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(MM
       // 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(MM
     }
   } 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
   
   // 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;
 }

Modified: cfe/trunk/test/Modules/Inputs/submodules/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/submodules/module.map?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/submodules/module.map (original)
+++ cfe/trunk/test/Modules/Inputs/submodules/module.map Tue Dec 17 04:31:37 2013
@@ -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" }
+}

Modified: cfe/trunk/test/Modules/Inputs/unnecessary-module-map-parsing/module.map
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/unnecessary-module-map-parsing/module.map?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/test/Modules/Inputs/unnecessary-module-map-parsing/module.map (original)
+++ cfe/trunk/test/Modules/Inputs/unnecessary-module-map-parsing/module.map Tue Dec 17 04:31:37 2013
@@ -1,3 +1,3 @@
 module a {
-  header "unknown.h"
+  eader "unknown.h"
 }

Modified: cfe/trunk/test/Modules/submodules.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/submodules.cpp?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/test/Modules/submodules.cpp (original)
+++ cfe/trunk/test/Modules/submodules.cpp Tue Dec 17 04:31:37 2013
@@ -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 at import-self-d.h:1 {{here}}
+
+// expected-error at Inputs/submodules/module.map:15{{header 'missing.h' not found}}
+ at import missing_headers.missing;
+ at import missing_headers.not_missing;
+void f() { NotMissingFunction(); };

Modified: cfe/trunk/test/Modules/unnecessary-module-map-parsing.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/unnecessary-module-map-parsing.c?rev=197485&r1=197484&r2=197485&view=diff
==============================================================================
--- cfe/trunk/test/Modules/unnecessary-module-map-parsing.c (original)
+++ cfe/trunk/test/Modules/unnecessary-module-map-parsing.c Tue Dec 17 04:31:37 2013
@@ -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"





More information about the cfe-commits mailing list