llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang @llvm/pr-subscribers-clang-modules Author: Cyndy Ishida (cyndyishida) <details> <summary>Changes</summary> PCHs (but also implicit modules generated from several implicit invocations like swiftc) previously reported a confusing diagnostic about module caches being mismatched by subdir. This is an implementation detail of the module machinery, and not very useful to the end user. Instead, report this case as a configuration mismatch when the compiler can confirm the module cache was passed the same between the current TU & previously compiled products. Ideally, each argument that could result in this error would be uniquely reported (e.g., O3), but as a starting point, providing something more general is strictly better than pointing the user to the module cache. This patch also includes NFCs for renaming variable names from Module to AST and formatting cleanup in related areas. resolves: rdar://167453135 --- Full diff: https://github.com/llvm/llvm-project/pull/174260.diff 10 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticFrontendKinds.td (+2-5) - (modified) clang/include/clang/Basic/DiagnosticSerializationKinds.td (+6) - (modified) clang/include/clang/Serialization/ASTReader.h (+2-1) - (modified) clang/lib/Frontend/CompilerInstance.cpp (+7-5) - (modified) clang/lib/Frontend/FrontendAction.cpp (+2-1) - (modified) clang/lib/Serialization/ASTReader.cpp (+33-22) - (modified) clang/test/Modules/ignored_macros.m (+1-1) - (modified) clang/test/Modules/mismatch-diagnostics.cpp (+1-1) - (added) clang/test/Modules/precompiled-config-mismatch-diagnostics.c (+49) - (modified) clang/test/PCH/module-hash-difference.m (+1-1) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticFrontendKinds.td b/clang/include/clang/Basic/DiagnosticFrontendKinds.td index 66c7f92622d1e..e2b257ceae80d 100644 --- a/clang/include/clang/Basic/DiagnosticFrontendKinds.td +++ b/clang/include/clang/Basic/DiagnosticFrontendKinds.td @@ -217,11 +217,8 @@ def note_incompatible_analyzer_plugin_api : Note< def err_module_build_requires_fmodules : Error< "module compilation requires '-fmodules'">; -def err_module_interface_requires_cpp_modules : Error< - "module interface compilation requires '-std=c++20'">; -def warn_module_config_mismatch : Warning< - "module file %0 cannot be loaded due to a configuration mismatch with the current " - "compilation">, InGroup<DiagGroup<"module-file-config-mismatch">>, DefaultError; +def err_module_interface_requires_cpp_modules + : Error<"module interface compilation requires '-std=c++20'">; def err_module_map_not_found : Error<"module map file '%0' not found">, DefaultFatal; def err_missing_module_name : Error< diff --git a/clang/include/clang/Basic/DiagnosticSerializationKinds.td b/clang/include/clang/Basic/DiagnosticSerializationKinds.td index b80aff385e01f..229f0bacbd796 100644 --- a/clang/include/clang/Basic/DiagnosticSerializationKinds.td +++ b/clang/include/clang/Basic/DiagnosticSerializationKinds.td @@ -39,6 +39,12 @@ def err_ast_file_targetopt_feature_mismatch : Error< "not">; def err_ast_file_langopt_mismatch : Error<"%0 was %select{disabled|enabled}1 in " "precompiled file '%3' but is currently %select{disabled|enabled}2">; +def warn_ast_file_config_mismatch + : Warning<"precompiled file '%0' cannot be loaded due to a configuration " + "mismatch with the current " + "compilation">, + InGroup<DiagGroup<"module-file-config-mismatch">>, + DefaultError; def err_ast_file_langopt_value_mismatch : Error< "%0 differs in precompiled file '%1' vs. current file">; def err_ast_file_codegenopt_mismatch : Error<"%0 was %select{disabled|enabled}1 in " diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index a720e5aca444b..2b3fa6d52f163 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -2024,7 +2024,8 @@ class ASTReader StringRef Filename, FileManager &FileMgr, const ModuleCache &ModCache, const PCHContainerReader &PCHContainerRdr, const LangOptions &LangOpts, const CodeGenOptions &CGOpts, const TargetOptions &TargetOpts, - const PreprocessorOptions &PPOpts, StringRef ExistingModuleCachePath, + const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts, + StringRef ExistingModuleCachePath, bool RequireStrictOptionMatches = false); /// Returns the suggested contents of the predefines buffer, diff --git a/clang/lib/Frontend/CompilerInstance.cpp b/clang/lib/Frontend/CompilerInstance.cpp index 39e20f371dcab..9d0c3048cc570 100644 --- a/clang/lib/Frontend/CompilerInstance.cpp +++ b/clang/lib/Frontend/CompilerInstance.cpp @@ -41,6 +41,7 @@ #include "clang/Serialization/GlobalModuleIndex.h" #include "clang/Serialization/InMemoryModuleCache.h" #include "clang/Serialization/ModuleCache.h" +#include "clang/Serialization/SerializationDiagnostic.h" #include "llvm/ADT/IntrusiveRefCntPtr.h" #include "llvm/ADT/STLExtras.h" #include "llvm/ADT/ScopeExit.h" @@ -1674,9 +1675,9 @@ bool CompilerInstance::loadModuleFile( // If -Wmodule-file-config-mismatch is mapped as an error or worse, allow the // ASTReader to diagnose it, since it can produce better errors that we can. bool ConfigMismatchIsRecoverable = - getDiagnostics().getDiagnosticLevel(diag::warn_module_config_mismatch, - SourceLocation()) - <= DiagnosticsEngine::Warning; + getDiagnostics().getDiagnosticLevel(diag::warn_ast_file_config_mismatch, + SourceLocation()) <= + DiagnosticsEngine::Warning; auto Listener = std::make_unique<ReadModuleNames>(*PP); auto &ListenerRef = *Listener; @@ -1696,7 +1697,8 @@ bool CompilerInstance::loadModuleFile( case ASTReader::ConfigurationMismatch: // Ignore unusable module files. - getDiagnostics().Report(SourceLocation(), diag::warn_module_config_mismatch) + getDiagnostics().Report(SourceLocation(), + diag::warn_ast_file_config_mismatch) << FileName; // All modules provided by any files we tried and failed to load are now // unavailable; includes of those modules should now be handled textually. @@ -1848,7 +1850,7 @@ ModuleLoadResult CompilerInstance::findOrCompileModuleAndReadAST( // FIXME: We shouldn't be setting HadFatalFailure below if we only // produce a warning here! getDiagnostics().Report(SourceLocation(), - diag::warn_module_config_mismatch) + diag::warn_ast_file_config_mismatch) << ModuleFilename; // Fall through to error out. [[fallthrough]]; diff --git a/clang/lib/Frontend/FrontendAction.cpp b/clang/lib/Frontend/FrontendAction.cpp index 1b56718b10862..cd404244ac379 100644 --- a/clang/lib/Frontend/FrontendAction.cpp +++ b/clang/lib/Frontend/FrontendAction.cpp @@ -1039,7 +1039,8 @@ bool FrontendAction::BeginSourceFile(CompilerInstance &CI, Dir->path(), FileMgr, CI.getModuleCache(), CI.getPCHContainerReader(), CI.getLangOpts(), CI.getCodeGenOpts(), CI.getTargetOpts(), - CI.getPreprocessorOpts(), SpecificModuleCachePath, + CI.getPreprocessorOpts(), CI.getHeaderSearchOpts(), + SpecificModuleCachePath, /*RequireStrictOptionMatches=*/true)) { PPOpts.ImplicitPCHInclude = std::string(Dir->path()); Found = true; diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 57771aad0c317..613e01ba0a5e7 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -960,13 +960,12 @@ bool SimpleASTReaderListener::ReadPreprocessorOptions( /// /// \param Diags If non-null, produce diagnostics for any mismatches incurred. /// \returns true when the module cache paths differ. -static bool checkModuleCachePath(llvm::vfs::FileSystem &VFS, - StringRef SpecificModuleCachePath, - StringRef ExistingModuleCachePath, - StringRef ModuleFilename, - DiagnosticsEngine *Diags, - const LangOptions &LangOpts, - const PreprocessorOptions &PPOpts) { +static bool checkModuleCachePath( + llvm::vfs::FileSystem &VFS, StringRef SpecificModuleCachePath, + StringRef ExistingModuleCachePath, StringRef ASTFilename, + DiagnosticsEngine *Diags, const LangOptions &LangOpts, + const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts, + const HeaderSearchOptions &ASTFileHSOpts) { if (!LangOpts.Modules || PPOpts.AllowPCHWithDifferentModulesCachePath || SpecificModuleCachePath == ExistingModuleCachePath) return false; @@ -974,21 +973,31 @@ static bool checkModuleCachePath(llvm::vfs::FileSystem &VFS, VFS.equivalent(SpecificModuleCachePath, ExistingModuleCachePath); if (EqualOrErr && *EqualOrErr) return false; - if (Diags) - Diags->Report(diag::err_ast_file_modulecache_mismatch) - << SpecificModuleCachePath << ExistingModuleCachePath << ModuleFilename; + if (Diags) { + // If the module cache arguments provided from the command line are the + // same, the mismatch must come from other arguments of the configuration + // and not directly the cache path. + EqualOrErr = + VFS.equivalent(ASTFileHSOpts.ModuleCachePath, HSOpts.ModuleCachePath); + if (EqualOrErr && *EqualOrErr) + Diags->Report(clang::diag::warn_ast_file_config_mismatch) << ASTFilename; + else + Diags->Report(diag::err_ast_file_modulecache_mismatch) + << SpecificModuleCachePath << ExistingModuleCachePath << ASTFilename; + } return true; } bool PCHValidator::ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, + StringRef ASTFilename, StringRef SpecificModuleCachePath, bool Complain) { + const HeaderSearch &HeaderSearchInfo = PP.getHeaderSearchInfo(); return checkModuleCachePath( Reader.getFileManager().getVirtualFileSystem(), SpecificModuleCachePath, - PP.getHeaderSearchInfo().getModuleCachePath(), ModuleFilename, + HeaderSearchInfo.getModuleCachePath(), ASTFilename, Complain ? &Reader.Diags : nullptr, PP.getLangOpts(), - PP.getPreprocessorOpts()); + PP.getPreprocessorOpts(), HeaderSearchInfo.getHeaderSearchOpts(), HSOpts); } void PCHValidator::ReadCounter(const ModuleFile &M, uint32_t Value) { @@ -5763,6 +5772,7 @@ namespace { const CodeGenOptions &ExistingCGOpts; const TargetOptions &ExistingTargetOpts; const PreprocessorOptions &ExistingPPOpts; + const HeaderSearchOptions &ExistingHSOpts; std::string ExistingModuleCachePath; FileManager &FileMgr; bool StrictOptionMatches; @@ -5772,11 +5782,12 @@ namespace { const CodeGenOptions &ExistingCGOpts, const TargetOptions &ExistingTargetOpts, const PreprocessorOptions &ExistingPPOpts, + const HeaderSearchOptions &ExistingHSOpts, StringRef ExistingModuleCachePath, FileManager &FileMgr, bool StrictOptionMatches) : ExistingLangOpts(ExistingLangOpts), ExistingCGOpts(ExistingCGOpts), ExistingTargetOpts(ExistingTargetOpts), - ExistingPPOpts(ExistingPPOpts), + ExistingPPOpts(ExistingPPOpts), ExistingHSOpts(ExistingHSOpts), ExistingModuleCachePath(ExistingModuleCachePath), FileMgr(FileMgr), StrictOptionMatches(StrictOptionMatches) {} @@ -5802,13 +5813,13 @@ namespace { } bool ReadHeaderSearchOptions(const HeaderSearchOptions &HSOpts, - StringRef ModuleFilename, + StringRef ASTFilename, StringRef SpecificModuleCachePath, bool Complain) override { - return checkModuleCachePath(FileMgr.getVirtualFileSystem(), - SpecificModuleCachePath, - ExistingModuleCachePath, ModuleFilename, - nullptr, ExistingLangOpts, ExistingPPOpts); + return checkModuleCachePath( + FileMgr.getVirtualFileSystem(), SpecificModuleCachePath, + ExistingModuleCachePath, ASTFilename, nullptr, ExistingLangOpts, + ExistingPPOpts, ExistingHSOpts, HSOpts); } bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts, @@ -6150,9 +6161,9 @@ bool ASTReader::isAcceptableASTFile( StringRef Filename, FileManager &FileMgr, const ModuleCache &ModCache, const PCHContainerReader &PCHContainerRdr, const LangOptions &LangOpts, const CodeGenOptions &CGOpts, const TargetOptions &TargetOpts, - const PreprocessorOptions &PPOpts, StringRef ExistingModuleCachePath, - bool RequireStrictOptionMatches) { - SimplePCHValidator validator(LangOpts, CGOpts, TargetOpts, PPOpts, + const PreprocessorOptions &PPOpts, const HeaderSearchOptions &HSOpts, + StringRef ExistingModuleCachePath, bool RequireStrictOptionMatches) { + SimplePCHValidator validator(LangOpts, CGOpts, TargetOpts, PPOpts, HSOpts, ExistingModuleCachePath, FileMgr, RequireStrictOptionMatches); return !readASTFileControlBlock(Filename, FileMgr, ModCache, PCHContainerRdr, diff --git a/clang/test/Modules/ignored_macros.m b/clang/test/Modules/ignored_macros.m index fe5a6f3206296..13e49dba57af3 100644 --- a/clang/test/Modules/ignored_macros.m +++ b/clang/test/Modules/ignored_macros.m @@ -10,7 +10,7 @@ // RUN: %clang_cc1 -fmodules-cache-path=%t.modules -fmodules -fimplicit-module-maps -I %S/Inputs -emit-pch -o %t.pch -x objective-c-header %s -verify // RUN: not %clang_cc1 -fmodules-cache-path=%t.modules -DIGNORED=1 -fmodules -fimplicit-module-maps -I %S/Inputs -include-pch %t.pch %s > %t.err 2>&1 // RUN: FileCheck -check-prefix=CHECK-CONFLICT %s < %t.err -// CHECK-CONFLICT: precompiled file '{{.*}}' was compiled with module cache path +// CHECK-CONFLICT: precompiled file '{{.*}}' cannot be loaded due to a configuration mismatch // Third trial: pass -DIGNORED=1 only to the second invocation, but // make it ignored. There should be no failure, IGNORED is defined in diff --git a/clang/test/Modules/mismatch-diagnostics.cpp b/clang/test/Modules/mismatch-diagnostics.cpp index be58f638cfa80..31d13fdd783f7 100644 --- a/clang/test/Modules/mismatch-diagnostics.cpp +++ b/clang/test/Modules/mismatch-diagnostics.cpp @@ -30,4 +30,4 @@ export module mismatching_module; //--- use.cpp import mismatching_module; // CHECK: error: POSIX thread support was enabled in precompiled file '{{.*[/|\\\\]}}mismatching_module.pcm' but is currently disabled -// CHECK-NEXT: module file {{.*[/|\\\\]}}mismatching_module.pcm cannot be loaded due to a configuration mismatch with the current compilation +// CHECK-NEXT: precompiled file '{{.*[/|\\\\]}}mismatching_module.pcm' cannot be loaded due to a configuration mismatch with the current compilation diff --git a/clang/test/Modules/precompiled-config-mismatch-diagnostics.c b/clang/test/Modules/precompiled-config-mismatch-diagnostics.c new file mode 100644 index 0000000000000..e5ad120700447 --- /dev/null +++ b/clang/test/Modules/precompiled-config-mismatch-diagnostics.c @@ -0,0 +1,49 @@ +// Validate configuration mismatches from precompiled files are reported. + +// RUN: rm -rf %t +// RUN: split-file %s %t + +// RUN: %clang_cc1 -I%t/BuildDir -fimplicit-module-maps -fmodules \ +// RUN: -fmodules-cache-path=%t/cache %t/h1.h -emit-pch -o %t/BuildDir/h1.h.pch + +// Check command line diff that is reported uniquely. +// RUN: not %clang_cc1 -I%t/BuildDir -fimplicit-module-maps -fmodules \ +// RUN: -O3 \ +// RUN: -fmodules-cache-path=%t/cache -fsyntax-only -include-pch %t/BuildDir/h1.h.pch \ +// RUN: %t/client.c 2>&1 | FileCheck %s --check-prefixes=OPTMODE,CONFIG + +// Check command line difference that end up in the module hash, but is not +// uniquely reported as a mismatch. +// RUN: not %clang_cc1 -I%t/BuildDir -fimplicit-module-maps -fmodules \ +// RUN: -dwarf-ext-refs -fmodule-format=obj \ +// RUN: -debug-info-kind=standalone -dwarf-version=5 \ +// RUN: -fmodules-cache-path=%t/cache -fsyntax-only -include-pch %t/BuildDir/h1.h.pch \ +// RUN: %t/client.c 2>&1 | FileCheck %s --check-prefix=CONFIG + +// Check that module cache path is uniquely reported. +// RUN: not %clang_cc1 -I%t/BuildDir -fimplicit-module-maps -fmodules \ +// RUN: -fmodules-cache-path=%t/wrong/cache -fsyntax-only \ +// RUN: -include-pch %t/BuildDir/h1.h.pch \ +// RUN: %t/client.c 2>&1 | FileCheck %s --check-prefix=CACHEPATH + +// OPTMODE: OptimizationLevel differs in precompiled file +// CONFIG: h1.h.pch' cannot be loaded due to a configuration mismatch +// CACHEPATH: h1.h.pch' was compiled with module cache path '{{.*}}', but the path is currently '{{.*}}' + +//--- BuildDir/A/module.modulemap +module A [system] { + umbrella "." +} + +//--- BuildDir/A/A.h +typedef int A_t; + +//--- h1.h +#include <A/A.h> +#if __OPTIMIZE__ +A_t foo(void); +#endif + +//--- client.c +typedef int foo_t; + diff --git a/clang/test/PCH/module-hash-difference.m b/clang/test/PCH/module-hash-difference.m index a48a15398903c..ff8432d3ba67d 100644 --- a/clang/test/PCH/module-hash-difference.m +++ b/clang/test/PCH/module-hash-difference.m @@ -4,5 +4,5 @@ // RUN: not %clang_cc1 -fsyntax-only -include-pch %t.pch %s -I %S/Inputs/modules -fmodules -fimplicit-module-maps -fmodules-cache-path=%t.mcp -fdisable-module-hash 2> %t.err // RUN: FileCheck -input-file=%t.err %s -// CHECK: error: precompiled file '{{.*}}' was compiled with module cache path {{.*}}, but the path is currently {{.*}} +// CHECK: error: precompiled file '{{.*}}' cannot be loaded due to a configuration mismatch with the current compilation @import Foo; `````````` </details> https://github.com/llvm/llvm-project/pull/174260 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
