[cfe-commits] r146933 - in /cfe/trunk: include/clang/Basic/DiagnosticFrontendKinds.td include/clang/Basic/DiagnosticGroups.td include/clang/Basic/Module.h lib/Frontend/CompilerInstance.cpp lib/Lex/PPDirectives.cpp lib/Serialization/ASTReader.cpp lib/Serialization/ASTWriter.cpp test/Modules/Inputs/Module.framework/Headers/NotInModule.h test/Modules/auto-module-import.m

Douglas Gregor dgregor at apple.com
Mon Dec 19 16:28:52 PST 2011


Author: dgregor
Date: Mon Dec 19 18:28:52 2011
New Revision: 146933

URL: http://llvm.org/viewvc/llvm-project?rev=146933&view=rev
Log:
Detect when mapping a #include/#import over to a submodule ends up
hitting a submodule that was never actually created, e.g., because
that header wasn't parsed. In such cases, complain (because the
module's umbrella headers don't cover everything) and fall back to
including the header.

Later, we'll add a warning at module-build time to catch all such
cases. However, this fallback is important to eliminate assertions in
the ASTWriter when this happens.


Added:
    cfe/trunk/test/Modules/Inputs/Module.framework/Headers/NotInModule.h
Modified:
    cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
    cfe/trunk/include/clang/Basic/DiagnosticGroups.td
    cfe/trunk/include/clang/Basic/Module.h
    cfe/trunk/lib/Frontend/CompilerInstance.cpp
    cfe/trunk/lib/Lex/PPDirectives.cpp
    cfe/trunk/lib/Serialization/ASTReader.cpp
    cfe/trunk/lib/Serialization/ASTWriter.cpp
    cfe/trunk/test/Modules/auto-module-import.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticFrontendKinds.td Mon Dec 19 18:28:52 2011
@@ -139,6 +139,8 @@
 def err_no_submodule : Error<"no submodule named %0 in module '%1'">;
 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 err_module_map_temp_file : Error<
   "unable to write temporary module map file '%0'">, DefaultFatal;
 }

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Dec 19 18:28:52 2011
@@ -88,6 +88,7 @@
 def IgnoredQualifiers : DiagGroup<"ignored-qualifiers">;
 def : DiagGroup<"import">;
 def IncompatiblePointerTypes : DiagGroup<"incompatible-pointer-types">;
+def IncompleteUmbrella : DiagGroup<"incomplete-umbrella">;
 def : DiagGroup<"init-self">;
 def : DiagGroup<"inline">;
 def : DiagGroup<"int-to-pointer-cast">;

Modified: cfe/trunk/include/clang/Basic/Module.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/Module.h (original)
+++ cfe/trunk/include/clang/Basic/Module.h Mon Dec 19 18:28:52 2011
@@ -58,6 +58,9 @@
   /// \brief The headers that are part of this module.
   llvm::SmallVector<const FileEntry *, 2> Headers;
   
+  /// \brief Whether this module was loaded from a module file.
+  unsigned IsFromModuleFile : 1;
+  
   /// \brief Whether this is a framework module.
   unsigned IsFramework : 1;
   
@@ -131,17 +134,18 @@
   explicit Module(StringRef Name, SourceLocation DefinitionLoc,
                   bool IsFramework)
     : Name(Name), DefinitionLoc(DefinitionLoc), Parent(0), Umbrella(),
-      IsFramework(IsFramework), IsExplicit(false), InferSubmodules(false),
-      InferExplicitSubmodules(false), InferExportWildcard(false),
-      NameVisibility(Hidden) { }
+      IsFromModuleFile(false), IsFramework(IsFramework), IsExplicit(false),
+      InferSubmodules(false), InferExplicitSubmodules(false),
+      InferExportWildcard(false), NameVisibility(Hidden) { }
   
   /// \brief Construct  a new module or submodule.
   Module(StringRef Name, SourceLocation DefinitionLoc, Module *Parent, 
          bool IsFramework, bool IsExplicit)
     : Name(Name), DefinitionLoc(DefinitionLoc), Parent(Parent), 
-      Umbrella(), IsFramework(IsFramework), IsExplicit(IsExplicit), 
-      InferSubmodules(false), InferExplicitSubmodules(false), 
-      InferExportWildcard(false),NameVisibility(Hidden) { }
+      Umbrella(), IsFromModuleFile(false), IsFramework(IsFramework), 
+      IsExplicit(IsExplicit), InferSubmodules(false), 
+      InferExplicitSubmodules(false), InferExportWildcard(false),
+      NameVisibility(Hidden) { }
   
   ~Module();
   

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Mon Dec 19 18:28:52 2011
@@ -1270,9 +1270,23 @@
   
   // Make the named module visible, if it's not already part of the module
   // we are parsing.
-  if (ModuleName != getLangOpts().CurrentModule)
+  if (ModuleName != getLangOpts().CurrentModule) {
+    if (!Module->IsFromModuleFile) {
+      // We have an umbrella header or directory that doesn't actually include
+      // all of the headers within the directory it covers. Complain about
+      // this missing submodule and recover by forgetting that we ever saw
+      // this submodule.
+      // FIXME: Should we detect this at module load time? It seems fairly
+      // expensive (and rare).
+      getDiagnostics().Report(ImportLoc, diag::warn_missing_submodule)
+        << Module->getFullModuleName()
+        << SourceRange(Path.front().second, Path.back().second);
+      
+      return 0;
+    }
     ModuleManager->makeModuleVisible(Module, Visibility);
-
+  }
+  
   // If this module import was due to an inclusion directive, create an 
   // implicit import declaration to capture it in the AST.
   if (IsInclusionDirective && hasASTContext()) {

Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
+++ cfe/trunk/lib/Lex/PPDirectives.cpp Mon Dec 19 18:28:52 2011
@@ -1388,11 +1388,12 @@
     // If this was an #__include_macros directive, only make macros visible.
     Module::NameVisibilityKind Visibility 
       = (IncludeKind == 3)? Module::MacrosVisible : Module::AllVisible;
-    TheModuleLoader.loadModule(IncludeTok.getLocation(), Path, Visibility,
-                               /*IsIncludeDirective=*/true);
+    Module *Imported
+      = TheModuleLoader.loadModule(IncludeTok.getLocation(), Path, Visibility,
+                                   /*IsIncludeDirective=*/true);
     
     // If this header isn't part of the module we're building, we're done.
-    if (!BuildingImportedModule)
+    if (!BuildingImportedModule && Imported)
       return;
   }
   

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Dec 19 18:28:52 2011
@@ -3086,6 +3086,7 @@
         return Failure;
       }
       
+      CurrentModule->IsFromModuleFile = true;
       CurrentModule->InferSubmodules = InferSubmodules;
       CurrentModule->InferExplicitSubmodules = InferExplicitSubmodules;
       CurrentModule->InferExportWildcard = InferExportWildcard;

Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTWriter.cpp?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTWriter.cpp Mon Dec 19 18:28:52 2011
@@ -1877,7 +1877,6 @@
   for (ASTContext::import_iterator I = Context->local_import_begin(),
                                 IEnd = Context->local_import_end();
        I != IEnd; ++I) {
-    assert(SubmoduleIDs.find(I->getImportedModule()) != SubmoduleIDs.end());
     if (Module *ImportedFrom
           = ModMap.inferModuleFromLocation(FullSourceLoc(I->getLocation(), 
                                                          SrcMgr))) {

Added: cfe/trunk/test/Modules/Inputs/Module.framework/Headers/NotInModule.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/Module.framework/Headers/NotInModule.h?rev=146933&view=auto
==============================================================================
--- cfe/trunk/test/Modules/Inputs/Module.framework/Headers/NotInModule.h (added)
+++ cfe/trunk/test/Modules/Inputs/Module.framework/Headers/NotInModule.h Mon Dec 19 18:28:52 2011
@@ -0,0 +1 @@
+int not_in_module;

Modified: cfe/trunk/test/Modules/auto-module-import.m
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/auto-module-import.m?rev=146933&r1=146932&r2=146933&view=diff
==============================================================================
--- cfe/trunk/test/Modules/auto-module-import.m (original)
+++ cfe/trunk/test/Modules/auto-module-import.m Mon Dec 19 18:28:52 2011
@@ -62,3 +62,12 @@
 int getNoUmbrellaAPrivate() { return no_umbrella_A_private; }
 
 int getNoUmbrellaBPrivateFail() { return no_umbrella_B_private; } // expected-error{{use of undeclared identifier 'no_umbrella_B_private'; did you mean 'no_umbrella_A_private'?}}
+
+// Test inclusion of headers that are under an umbrella directory but
+// not actually part of the module.
+#include <Module/NotInModule.h> // expected-warning{{treating #include as an import of module 'Module.NotInModule'}} \
+  // expected-warning{{missing submodule 'Module.NotInModule'}}
+
+int getNotInModule() {
+  return not_in_module;
+}





More information about the cfe-commits mailing list