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

Daniel Jasper via cfe-commits cfe-commits at lists.llvm.org
Sun Dec 4 14:45:02 PST 2016


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 at 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&view=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&r1=288448&r2=288449&view=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<std::pair<IdentifierInf
>
>  /// \brief Describes the result of attempting to load a module.
>  class ModuleLoadResult {
> -  llvm::PointerIntPair<Module *, 1, bool> 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<Module *, 2, LoadResultKind> 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&r1=288448&r2=288449&view=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<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();
>      }
> @@ -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 = ModuleLoadResult(Module, false);
> +  LastModuleImportResult = ModuleLoadResult(Module);
>    return LastModuleImportResult;
>  }
>
>
> Modified: cfe/trunk/lib/Lex/PPDirectives.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/
> PPDirectives.cpp?rev=288449&r1=288448&r2=288449&view=diff
> ============================================================
> ==================
> --- cfe/trunk/lib/Lex/PPDirectives.cpp (original)
> +++ cfe/trunk/lib/Lex/PPDirectives.cpp Thu Dec  1 19:52:28 2016
> @@ -1866,10 +1866,7 @@ void Preprocessor::HandleIncludeDirectiv
>      // unavailable, diagnose the situation and bail out.
>      // FIXME: Remove this; loadModule does the same check (but produces
>      // slightly worse diagnostics).
> -    if (!SuggestedModule.getModule()->isAvailable() &&
> -        !SuggestedModule.getModule()
> -             ->getTopLevelModule()
> -             ->HasIncompatibleModuleFile) {
> +    if (!SuggestedModule.getModule()->isAvailable()) {
>        Module::Requirement Requirement;
>        Module::UnresolvedHeaderDirective MissingHeader;
>        Module *M = SuggestedModule.getModule();
> @@ -1918,9 +1915,12 @@ void Preprocessor::HandleIncludeDirectiv
>      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 exists for it (because the umbrella header is
> +      // actual module containing it exists (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()) {
>
> Added: cfe/trunk/test/Modules/config-mismatch.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
> Modules/config-mismatch.cpp?rev=288449&view=auto
> ============================================================
> ==================
> --- cfe/trunk/test/Modules/config-mismatch.cpp (added)
> +++ cfe/trunk/test/Modules/config-mismatch.cpp Thu Dec  1 19:52:28 2016
> @@ -0,0 +1,10 @@
> +// 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"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20161204/108ef464/attachment-0001.html>


More information about the cfe-commits mailing list