Re: r288449 - Recover better from an incompatible .pcm file being provided by -fmodule-file=.

[Daniel Jasper via cfe-commits]

This is unfortunately causing problems as is, as it can change the
diagnostic that's created when the include of a module with config-mismatch
is inside a namespace. I have tried to fix this for a bit, but I am not
sure what the right solution is. For now, I have reverted this in r288626
and left the details of my investigation on that patch description.

On Fri, Dec 2, 2016 at 2:52 AM, Richard Smith via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> Author: rsmith
> Date: Thu Dec  1 19:52:28 2016
> New Revision: 288449
>
> URL: http://llvm.org/viewvc/llvm-project?rev=288449=rev
> Log:
> Recover better from an incompatible .pcm file being provided by
> -fmodule-file=.
> We try to include the headers of the module textually in this case, still
> enforcing the modules semantic rules. In order to make that work, we need
> to
> still track that we're entering and leaving the module. Also, if the
> module was
> also marked as unavailable (perhaps because it was missing a file), we
> shouldn't mark the module unavailable -- we don't need the module to be
> complete if we're going to enter it textually.
>
> Added:
> cfe/trunk/test/Modules/config-mismatch.cpp
> Modified:
> cfe/trunk/include/clang/Lex/ModuleLoader.h
> cfe/trunk/lib/Frontend/CompilerInstance.cpp
> cfe/trunk/lib/Lex/PPDirectives.cpp
>
> Modified: cfe/trunk/include/clang/Lex/ModuleLoader.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
> clang/Lex/ModuleLoader.h?rev=288449=288448=288449=diff
> 
> ==
> --- cfe/trunk/include/clang/Lex/ModuleLoader.h (original)
> +++ cfe/trunk/include/clang/Lex/ModuleLoader.h Thu Dec  1 19:52:28 2016
> @@ -31,13 +31,22 @@ typedef ArrayRef  /// \brief Describes the result of attempting to load a module.
>  class ModuleLoadResult {
> -  llvm::PointerIntPair Storage;
> -
>  public:
> -  ModuleLoadResult() : Storage() { }
> +  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 Storage;
>
> -  ModuleLoadResult(Module *module, bool missingExpected)
> -: Storage(module, missingExpected) { }
> +  ModuleLoadResult() : Storage() { }
> +  ModuleLoadResult(Module *M) : Storage(M, Normal) {}
> +  ModuleLoadResult(LoadResultKind Kind) : Storage(nullptr, Kind) {}
>
>operator Module *() const { return Storage.getPointer(); }
>
> @@ -45,7 +54,11 @@ 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(); }
> +  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; }
>  };
>
>  /// \brief Abstract interface for a module loader.
>
> Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/
> Frontend/CompilerInstance.cpp?rev=288449=288448=288449=diff
> 
> ==
> --- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
> +++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu Dec  1 19:52:28 2016
> @@ -1393,8 +1393,21 @@ bool CompilerInstance::loadModuleFile(St
>  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 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();
>  }
> @@ -1498,7 +1511,7 @@ CompilerInstance::loadModule(SourceLocat
>if (Module && Module->HasIncompatibleModuleFile) {
>  

r288449 - Recover better from an incompatible .pcm file being provided by -fmodule-file=.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Dec  1 19:52:28 2016
New Revision: 288449

URL: http://llvm.org/viewvc/llvm-project?rev=288449=rev
Log:
Recover better from an incompatible .pcm file being provided by -fmodule-file=.
We try to include the headers of the module textually in this case, still
enforcing the modules semantic rules. In order to make that work, we need to
still track that we're entering and leaving the module. Also, if the module was
also marked as unavailable (perhaps because it was missing a file), we
shouldn't mark the module unavailable -- we don't need the module to be
complete if we're going to enter it textually.

Added:
cfe/trunk/test/Modules/config-mismatch.cpp
Modified:
cfe/trunk/include/clang/Lex/ModuleLoader.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Lex/PPDirectives.cpp

Modified: cfe/trunk/include/clang/Lex/ModuleLoader.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleLoader.h?rev=288449=288448=288449=diff
==
--- cfe/trunk/include/clang/Lex/ModuleLoader.h (original)
+++ cfe/trunk/include/clang/Lex/ModuleLoader.h Thu Dec  1 19:52:28 2016
@@ -31,13 +31,22 @@ typedef ArrayRefgetName()))
+.findModule(II->getName())) {
   M->HasIncompatibleModuleFile = true;
+
+  // Mark module as available if the only reason it was unavailable
+  // was missing headers.
+  SmallVector 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();
 }
@@ -1498,7 +1511,7 @@ CompilerInstance::loadModule(SourceLocat
   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(nullptr, /*missingExpected*/true);
+return ModuleLoadResult::ConfigMismatch;
   }
 
   getDiagnostics().Report(ModuleNameLoc, diag::err_module_build_disabled)
@@ -1705,7 +1718,7 @@ CompilerInstance::loadModule(SourceLocat
 << Module->getFullModuleName()
 << SourceRange(Path.front().second, Path.back().second);
 
-  return ModuleLoadResult(nullptr, true);
+  return ModuleLoadResult::MissingExpected;
 }
 
 // Check whether this module is available.
@@ -1739,7 +1752,7 @@ CompilerInstance::loadModule(SourceLocat
   }
 
   LastModuleImportLoc = ImportLoc;
-  LastModuleImportResult