From fc454719b46cd00a5877958cd1cb9097e4f0472c Mon Sep 17 00:00:00 2001 From: Daniel Jasper Date: Sun, 4 Dec 2016 22:34:37 +0000 Subject: [PATCH] Revert "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." 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). git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@288626 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/clang/Lex/ModuleLoader.h | 25 ++++++------------------- lib/Frontend/CompilerInstance.cpp | 21 ++++----------------- lib/Lex/PPDirectives.cpp | 10 +++++----- test/Modules/config-mismatch.cpp | 10 ---------- 4 files changed, 15 insertions(+), 51 deletions(-) delete mode 100644 test/Modules/config-mismatch.cpp diff --git a/include/clang/Lex/ModuleLoader.h b/include/clang/Lex/ModuleLoader.h index 70770d17e9..ae79650d1f 100644 --- a/include/clang/Lex/ModuleLoader.h +++ b/include/clang/Lex/ModuleLoader.h @@ -31,22 +31,13 @@ typedef ArrayRef > 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 Storage; + llvm::PointerIntPair 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. diff --git a/lib/Frontend/CompilerInstance.cpp b/lib/Frontend/CompilerInstance.cpp index 991e9ef990..d314f6d18a 100644 --- a/lib/Frontend/CompilerInstance.cpp +++ b/lib/Frontend/CompilerInstance.cpp @@ -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 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; } diff --git a/lib/Lex/PPDirectives.cpp b/lib/Lex/PPDirectives.cpp index 85504de3d1..7fc008274b 100644 --- a/lib/Lex/PPDirectives.cpp +++ b/lib/Lex/PPDirectives.cpp @@ -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/test/Modules/config-mismatch.cpp b/test/Modules/config-mismatch.cpp deleted file mode 100644 index 548e78452d..0000000000 --- a/test/Modules/config-mismatch.cpp +++ /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 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" -- 2.40.0