<div dir="ltr">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.</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Dec 2, 2016 at 2:52 AM, Richard Smith via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: rsmith<br>
Date: Thu Dec 1 19:52:28 2016<br>
New Revision: 288449<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=288449&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project?rev=288449&view=rev</a><br>
Log:<br>
Recover better from an incompatible .pcm file being provided by -fmodule-file=.<br>
We try to include the headers of the module textually in this case, still<br>
enforcing the modules semantic rules. In order to make that work, we need to<br>
still track that we're entering and leaving the module. Also, if the module was<br>
also marked as unavailable (perhaps because it was missing a file), we<br>
shouldn't mark the module unavailable -- we don't need the module to be<br>
complete if we're going to enter it textually.<br>
<br>
Added:<br>
cfe/trunk/test/Modules/config-<wbr>mismatch.cpp<br>
Modified:<br>
cfe/trunk/include/clang/Lex/<wbr>ModuleLoader.h<br>
cfe/trunk/lib/Frontend/<wbr>CompilerInstance.cpp<br>
cfe/trunk/lib/Lex/<wbr>PPDirectives.cpp<br>
<br>
Modified: cfe/trunk/include/clang/Lex/<wbr>ModuleLoader.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/ModuleLoader.h?rev=288449&r1=288448&r2=288449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/include/<wbr>clang/Lex/ModuleLoader.h?rev=<wbr>288449&r1=288448&r2=288449&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/include/clang/Lex/<wbr>ModuleLoader.h (original)<br>
+++ cfe/trunk/include/clang/Lex/<wbr>ModuleLoader.h Thu Dec 1 19:52:28 2016<br>
@@ -31,13 +31,22 @@ typedef ArrayRef<std::pair<<wbr>IdentifierInf<br>
<br>
/// \brief Describes the result of attempting to load a module.<br>
class ModuleLoadResult {<br>
- llvm::PointerIntPair<Module *, 1, bool> Storage;<br>
-<br>
public:<br>
- ModuleLoadResult() : Storage() { }<br>
+ enum LoadResultKind {<br>
+ // We either succeeded or failed to load the named module.<br>
+ Normal,<br>
+ // The module exists, but does not actually contain the named submodule.<br>
+ // This should only happen if the named submodule was inferred from an<br>
+ // umbrella directory, but not actually part of the umbrella header.<br>
+ MissingExpected,<br>
+ // The module exists but cannot be imported due to a configuration mismatch.<br>
+ ConfigMismatch<br>
+ };<br>
+ llvm::PointerIntPair<Module *, 2, LoadResultKind> Storage;<br>
<br>
- ModuleLoadResult(Module *module, bool missingExpected)<br>
- : Storage(module, missingExpected) { }<br>
+ ModuleLoadResult() : Storage() { }<br>
+ ModuleLoadResult(Module *M) : Storage(M, Normal) {}<br>
+ ModuleLoadResult(<wbr>LoadResultKind Kind) : Storage(nullptr, Kind) {}<br>
<br>
operator Module *() const { return Storage.getPointer(); }<br>
<br>
@@ -45,7 +54,11 @@ public:<br>
/// actually a submodule that we expected to see (based on implying the<br>
/// submodule from header structure), but didn't materialize in the actual<br>
/// module.<br>
- bool isMissingExpected() const { return Storage.getInt(); }<br>
+ bool isMissingExpected() const { return Storage.getInt() == MissingExpected; }<br>
+<br>
+ /// \brief Determines whether the module failed to load due to a configuration<br>
+ /// mismatch with an explicitly-named .pcm file from the command line.<br>
+ bool isConfigMismatch() const { return Storage.getInt() == ConfigMismatch; }<br>
};<br>
<br>
/// \brief Abstract interface for a module loader.<br>
<br>
Modified: cfe/trunk/lib/Frontend/<wbr>CompilerInstance.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=288449&r1=288448&r2=288449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/<wbr>Frontend/CompilerInstance.cpp?<wbr>rev=288449&r1=288448&r2=<wbr>288449&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Frontend/<wbr>CompilerInstance.cpp (original)<br>
+++ cfe/trunk/lib/Frontend/<wbr>CompilerInstance.cpp Thu Dec 1 19:52:28 2016<br>
@@ -1393,8 +1393,21 @@ bool CompilerInstance::<wbr>loadModuleFile(St<br>
if (Module *M = CI.getPreprocessor()<br>
.getHeaderSearchInfo()<br>
.getModuleMap()<br>
- .findModule(II->getName()))<br>
+ .findModule(II->getName())) {<br>
M->HasIncompatibleModuleFile = true;<br>
+<br>
+ // Mark module as available if the only reason it was unavailable<br>
+ // was missing headers.<br>
+ SmallVector<Module *, 2> Stack;<br>
+ Stack.push_back(M);<br>
+ while (!Stack.empty()) {<br>
+ Module *Current = Stack.pop_back_val();<br>
+ if (Current-><wbr>IsMissingRequirement) continue;<br>
+ Current->IsAvailable = true;<br>
+ Stack.insert(Stack.end(),<br>
+ Current->submodule_begin(), Current->submodule_end());<br>
+ }<br>
+ }<br>
}<br>
LoadedModules.clear();<br>
}<br>
@@ -1498,7 +1511,7 @@ CompilerInstance::loadModule(<wbr>SourceLocat<br>
if (Module && Module-><wbr>HasIncompatibleModuleFile) {<br>
// We tried and failed to load a module file for this module. Fall<br>
// back to textual inclusion for its headers.<br>
- return ModuleLoadResult(nullptr, /*missingExpected*/true);<br>
+ return ModuleLoadResult::<wbr>ConfigMismatch;<br>
}<br>
<br>
getDiagnostics().Report(<wbr>ModuleNameLoc, diag::err_module_build_<wbr>disabled)<br>
@@ -1705,7 +1718,7 @@ CompilerInstance::loadModule(<wbr>SourceLocat<br>
<< Module->getFullModuleName()<br>
<< SourceRange(Path.front().<wbr>second, Path.back().second);<br>
<br>
- return ModuleLoadResult(nullptr, true);<br>
+ return ModuleLoadResult::<wbr>MissingExpected;<br>
}<br>
<br>
// Check whether this module is available.<br>
@@ -1739,7 +1752,7 @@ CompilerInstance::loadModule(<wbr>SourceLocat<br>
}<br>
<br>
LastModuleImportLoc = ImportLoc;<br>
- LastModuleImportResult = ModuleLoadResult(Module, false);<br>
+ LastModuleImportResult = ModuleLoadResult(Module);<br>
return LastModuleImportResult;<br>
}<br>
<br>
<br>
Modified: cfe/trunk/lib/Lex/<wbr>PPDirectives.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPDirectives.cpp?rev=288449&r1=288448&r2=288449&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/lib/Lex/<wbr>PPDirectives.cpp?rev=288449&<wbr>r1=288448&r2=288449&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/lib/Lex/<wbr>PPDirectives.cpp (original)<br>
+++ cfe/trunk/lib/Lex/<wbr>PPDirectives.cpp Thu Dec 1 19:52:28 2016<br>
@@ -1866,10 +1866,7 @@ void Preprocessor::<wbr>HandleIncludeDirectiv<br>
// unavailable, diagnose the situation and bail out.<br>
// FIXME: Remove this; loadModule does the same check (but produces<br>
// slightly worse diagnostics).<br>
- if (!SuggestedModule.getModule()-<wbr>>isAvailable() &&<br>
- !SuggestedModule.getModule()<br>
- ->getTopLevelModule()<br>
- ->HasIncompatibleModuleFile) {<br>
+ if (!SuggestedModule.getModule()-<wbr>>isAvailable()) {<br>
Module::Requirement Requirement;<br>
Module::<wbr>UnresolvedHeaderDirective MissingHeader;<br>
Module *M = SuggestedModule.getModule();<br>
@@ -1918,9 +1915,12 @@ void Preprocessor::<wbr>HandleIncludeDirectiv<br>
else if (Imported.isMissingExpected()) {<br>
// We failed to find a submodule that we assumed would exist (because it<br>
// was in the directory of an umbrella header, for instance), but no<br>
- // actual module exists for it (because the umbrella header is<br>
+ // actual module containing it exists (because the umbrella header is<br>
// incomplete). Treat this as a textual inclusion.<br>
SuggestedModule = ModuleMap::KnownHeader();<br>
+ } else if (Imported.isConfigMismatch()) {<br>
+ // On a configuration mismatch, enter the header textually. We still know<br>
+ // that it's part of the corresponding module.<br>
} else {<br>
// We hit an error processing the import. Bail out.<br>
if (hadModuleLoaderFatalFailure()<wbr>) {<br>
<br>
Added: cfe/trunk/test/Modules/config-<wbr>mismatch.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/config-mismatch.cpp?rev=288449&view=auto" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-<wbr>project/cfe/trunk/test/<wbr>Modules/config-mismatch.cpp?<wbr>rev=288449&view=auto</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- cfe/trunk/test/Modules/config-<wbr>mismatch.cpp (added)<br>
+++ cfe/trunk/test/Modules/config-<wbr>mismatch.cpp Thu Dec 1 19:52:28 2016<br>
@@ -0,0 +1,10 @@<br>
+// RUN: rm -rf %t<br>
+// RUN: mkdir %t<br>
+// RUN: echo 'module M { header "foo.h" header "bar.h" }' > %t/map<br>
+// RUN: echo 'template<typename T> void f(T t) { int n; t.f(n); }' > %t/foo.h<br>
+// RUN: touch %t/bar.h<br>
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-<wbr>visibility -x c++ %t/map -emit-module -fmodule-name=M -o %t/pcm<br>
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-<wbr>visibility -fmodule-map-file=%t/map -fmodule-file=%t/pcm -I%t %s -fsyntax-only -fexceptions -Wno-module-file-config-<wbr>mismatch<br>
+// RUN: rm %t/bar.h<br>
+// RUN: %clang_cc1 -fmodules -fmodules-local-submodule-<wbr>visibility -fmodule-map-file=%t/map -fmodule-file=%t/pcm -I%t %s -fsyntax-only -fexceptions -Wno-module-file-config-<wbr>mismatch<br>
+#include "foo.h"<br>
<br>
<br>
______________________________<wbr>_________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>