llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Cyndy Ishida (cyndyishida) <details> <summary>Changes</summary> cf8597bd3b87 introduced relocation checks for dependency scans but also disabled it when the modules were built in the same build session for implicit module builds. It turns out downstream projects (using implicit modules) can depend on the relocation check calling `lookupModule` for order-dependent module resolution to hide poorly modularized dependencies within the same build session, since that call loads discovered modules during search. Introduce `-fmodules-force-redundant-lookup` as an escape hatch to maintain preexisting redundant lookup for those projects while keeping the fast path the default for dependency scans. --- Full diff: https://github.com/llvm/llvm-project/pull/219107.diff 4 Files Affected: - (modified) clang/include/clang/Lex/PreprocessorOptions.h (+5) - (modified) clang/include/clang/Options/Options.td (+5) - (modified) clang/lib/Serialization/ASTReader.cpp (+7-4) - (added) clang/test/Modules/build-session-validation-force-redundant-lookup.c (+21) ``````````diff diff --git a/clang/include/clang/Lex/PreprocessorOptions.h b/clang/include/clang/Lex/PreprocessorOptions.h index 4d1a30712836f..10a8ee98f6782 100644 --- a/clang/include/clang/Lex/PreprocessorOptions.h +++ b/clang/include/clang/Lex/PreprocessorOptions.h @@ -84,6 +84,11 @@ class PreprocessorOptions { /// Perform extra checks when loading PCM files for mutable file systems. bool ModulesCheckRelocated = true; + /// Perform redundant module lookups. This is typically for + /// compilations that rely on side effects of module lookup due to + /// poor modularization. + bool ModulesForceRedundantLookup = false; + /// Initialize the preprocessor with the compiler and target specific /// predefines. bool UsePredefines = true; diff --git a/clang/include/clang/Options/Options.td b/clang/include/clang/Options/Options.td index eb5a009b5628c..bfa3dce59b4c2 100644 --- a/clang/include/clang/Options/Options.td +++ b/clang/include/clang/Options/Options.td @@ -9133,6 +9133,11 @@ def fno_modules_check_relocated Group<f_Group>, HelpText<"Skip checks for relocated modules when loading PCM files">, MarshallingInfoNegativeFlag<PreprocessorOpts<"ModulesCheckRelocated">>; +def fmodules_force_redundant_lookup + : Flag<["-"], "fmodules-force-redundant-lookup">, + Group<f_Group>, + HelpText<"Force redundant module lookup when loading PCM files">, + MarshallingInfoFlag<PreprocessorOpts<"ModulesForceRedundantLookup">>; def init_datetime_macros_EQ : Joined<["-"], "init-datetime-macros=">, HelpText<"Change __DATE__, __TIME__, and __TIMESTAMP__ macros initialization and expansion">, Values<"default,literalone,undefined">, diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index a11e774d7bb41..81448de0c8164 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -3225,10 +3225,13 @@ ASTReader::getModuleForRelocationChecks(ModuleFile &F, bool DirectoryCheck) { // session. auto [EnablesBSValidation, WasValidated] = wasValidatedInBuildSession(F, HSOpts); - if (WasValidated) - return {std::nullopt, IgnoreError}; - if (EnablesBSValidation && - static_cast<uint64_t>(F.ModTime) >= HSOpts.BuildSessionTimestamp) + const bool SkipModuleLookup = + !PP.getPreprocessorOpts().ModulesForceRedundantLookup && + (WasValidated || + (EnablesBSValidation && + static_cast<uint64_t>(F.ModTime) >= HSOpts.BuildSessionTimestamp)); + + if (SkipModuleLookup) return {std::nullopt, IgnoreError}; Diag(diag::remark_module_check_relocation) << F.ModuleName << F.FileName; diff --git a/clang/test/Modules/build-session-validation-force-redundant-lookup.c b/clang/test/Modules/build-session-validation-force-redundant-lookup.c new file mode 100644 index 0000000000000..5386393a6c861 --- /dev/null +++ b/clang/test/Modules/build-session-validation-force-redundant-lookup.c @@ -0,0 +1,21 @@ +// RUN: rm -rf %t +// RUN: split-file %s %t + +// Verify a module that is built in the same session was looked up during a relocation +// check when forced. + +// RUN: touch %t/session.timestamp +// RUN: %clang -fmodules -fimplicit-module-maps -fsyntax-only %t/tu1.c \ +// RUN: -fmodules-cache-path=%t/cache -I%t/include \ +// RUN: -fbuild-session-file=%t/session.timestamp -fmodules-validate-once-per-build-session \ +// RUN: -Xclang -fmodules-force-redundant-lookup -Rmodule-validation 2>&1 | FileCheck %s + +// CHECK: checking if module 'Dep' from '{{.*}}Dep-{{.*}}.pcm' has relocated + +//--- include/module.modulemap +module Dep { header "Dep.h" } +//--- include/Dep.h +int foo(void); + +//--- tu1.c +#include "Dep.h" `````````` </details> https://github.com/llvm/llvm-project/pull/219107 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
