Revert "Recover better from an incompatible .pcm file being provided by -fmodule...
authorDaniel Jasper <djasper@google.com>
Sun, 4 Dec 2016 22:34:37 +0000 (22:34 +0000)
committerDaniel Jasper <djasper@google.com>
Sun, 4 Dec 2016 22:34:37 +0000 (22:34 +0000)
This reverts commit r288449.

I believe that this is currently faulty wrt. modules being imported
inside namespaces. Adding these lines to the new test:

  namespace n {
  #include "foo.h"
  }

Makes it break with

  fatal error: import of module 'M' appears within namespace 'n'

However, I believe it should fail with

  error: redundant #include of module 'M' appears within namespace 'n'

I have tracked this down to us now inserting a tok::annot_module_begin
instead of a tok::annot_module_include in
Preprocessor::HandleIncludeDirective() and then later in
Parser::parseMisplacedModuleImport(), we hit the code path for
tok::annot_module_begin, which doesn't set FromInclude of
checkModuleImportContext to true (thus leading to the "wrong"
diagnostic).

llvm-svn: 288626

clang/include/clang/Lex/ModuleLoader.h
clang/lib/Frontend/CompilerInstance.cpp
clang/lib/Lex/PPDirectives.cpp
clang/test/Modules/config-mismatch.cpp [deleted file]

index 70770d1..ae79650 100644 (file)
@@ -31,22 +31,13 @@ typedef ArrayRef<std::pair<IdentifierInfo *, SourceLocation> > ModuleIdPath;
 
 /// \brief Describes the result of attempting to load a module.
 class ModuleLoadResult {
-public:
-  enum LoadResultKind {
-    // We either succeeded or failed to load the named module.
-    Normal,
-    // The module exists, but does not actually contain the named submodule.
-    // This should only happen if the named submodule was inferred from an
-    // umbrella directory, but not actually part of the umbrella header.
-    MissingExpected,
-    // The module exists but cannot be imported due to a configuration mismatch.
-    ConfigMismatch
-  };
-  llvm::PointerIntPair<Module *, 2, LoadResultKind> Storage;
+  llvm::PointerIntPair<Module *, 1, bool> Storage;
 
+public:
   ModuleLoadResult() : Storage() { }
-  ModuleLoadResult(Module *M) : Storage(M, Normal) {}
-  ModuleLoadResult(LoadResultKind Kind) : Storage(nullptr, Kind) {}
+
+  ModuleLoadResult(Module *module, bool missingExpected)
+    : Storage(module, missingExpected) { }
 
   operator Module *() const { return Storage.getPointer(); }
 
@@ -54,11 +45,7 @@ public:
   /// actually a submodule that we expected to see (based on implying the
   /// submodule from header structure), but didn't materialize in the actual
   /// module.
-  bool isMissingExpected() const { return Storage.getInt() == MissingExpected; }
-
-  /// \brief Determines whether the module failed to load due to a configuration
-  /// mismatch with an explicitly-named .pcm file from the command line.
-  bool isConfigMismatch() const { return Storage.getInt() == ConfigMismatch; }
+  bool isMissingExpected() const { return Storage.getInt(); }
 };
 
 /// \brief Abstract interface for a module loader.
index 991e9ef..d314f6d 100644 (file)
@@ -1393,21 +1393,8 @@ bool CompilerInstance::loadModuleFile(StringRef FileName) {
         if (Module *M = CI.getPreprocessor()
                             .getHeaderSearchInfo()
                             .getModuleMap()
-                            .findModule(II->getName())) {
+                            .findModule(II->getName()))
           M->HasIncompatibleModuleFile = true;
-
-          // Mark module as available if the only reason it was unavailable
-          // was missing headers.
-          SmallVector<Module *, 2> Stack;
-          Stack.push_back(M);
-          while (!Stack.empty()) {
-            Module *Current = Stack.pop_back_val();
-            if (Current->IsMissingRequirement) continue;
-            Current->IsAvailable = true;
-            Stack.insert(Stack.end(),
-                         Current->submodule_begin(), Current->submodule_end());
-          }
-        }
       }
       LoadedModules.clear();
     }
@@ -1511,7 +1498,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
       if (Module && Module->HasIncompatibleModuleFile) {
         // We tried and failed to load a module file for this module. Fall
         // back to textual inclusion for its headers.
-        return ModuleLoadResult::ConfigMismatch;
+        return ModuleLoadResult(nullptr, /*missingExpected*/true);
       }
 
       getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
@@ -1718,7 +1705,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
         << Module->getFullModuleName()
         << SourceRange(Path.front().second, Path.back().second);
 
-      return ModuleLoadResult::MissingExpected;
+      return ModuleLoadResult(nullptr, true);
     }
 
     // Check whether this module is available.
@@ -1752,7 +1739,7 @@ CompilerInstance::loadModule(SourceLocation ImportLoc,
   }
 
   LastModuleImportLoc = ImportLoc;
-  LastModuleImportResult = ModuleLoadResult(Module);
+  LastModuleImportResult = ModuleLoadResult(Module, false);
   return LastModuleImportResult;
 }
 
index 85504de..7fc0082 100644 (file)
@@ -1866,7 +1866,10 @@ 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()) {
+    if (!SuggestedModule.getModule()->isAvailable() &&
+        !SuggestedModule.getModule()
+             ->getTopLevelModule()
+             ->HasIncompatibleModuleFile) {
       Module::Requirement Requirement;
       Module::UnresolvedHeaderDirective MissingHeader;
       Module *M = SuggestedModule.getModule();
@@ -1915,12 +1918,9 @@ void Preprocessor::HandleIncludeDirective(SourceLocation HashLoc,
     else if (Imported.isMissingExpected()) {
       // We failed to find a submodule that we assumed would exist (because it
       // was in the directory of an umbrella header, for instance), but no
-      // actual module containing it exists (because the umbrella header is
+      // actual module exists for it (because the umbrella header is
       // incomplete).  Treat this as a textual inclusion.
       SuggestedModule = ModuleMap::KnownHeader();
-    } else if (Imported.isConfigMismatch()) {
-      // On a configuration mismatch, enter the header textually. We still know
-      // that it's part of the corresponding module.
     } else {
       // We hit an error processing the import. Bail out.
       if (hadModuleLoaderFatalFailure()) {
diff --git a/clang/test/Modules/config-mismatch.cpp b/clang/test/Modules/config-mismatch.cpp
deleted file mode 100644 (file)
index 548e784..0000000
+++ /dev/null
@@ -1,10 +0,0 @@
-// RUN: rm -rf %t
-// RUN: mkdir %t
-// RUN: echo 'module M { header "foo.h" header "bar.h" }' > %t/map
-// RUN: echo 'template<typename T> void f(T t) { int n; t.f(n); }' > %t/foo.h
-// RUN: touch %t/bar.h
-// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -x c++ %t/map -emit-module -fmodule-name=M -o %t/pcm
-// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fmodule-map-file=%t/map -fmodule-file=%t/pcm -I%t %s -fsyntax-only -fexceptions -Wno-module-file-config-mismatch
-// RUN: rm %t/bar.h
-// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-visibility -fmodule-map-file=%t/map -fmodule-file=%t/pcm -I%t %s -fsyntax-only -fexceptions -Wno-module-file-config-mismatch
-#include "foo.h"