diff --git a/Source/cmake.cxx b/Source/cmake.cxx index a98e915cfa..f33f61ab2e 100644 --- a/Source/cmake.cxx +++ b/Source/cmake.cxx @@ -293,13 +293,6 @@ bool cmakeCheckStampList(std::string const& stampList) return true; } -bool isDiagnosticSet(cmStateSnapshot const& state, - cmDiagnosticCategory category) -{ - constexpr cmDiagnosticAction unset = cmDiagnostics::Undefined; - return (state.GetDiagnostic(category, unset) == unset); -} - } // namespace cmDocumentationEntry cmake::CMAKE_STANDARD_OPTIONS_TABLE[15] = { @@ -513,22 +506,22 @@ void cmake::SetDiagnosticsFromPreset( auto const wi = warnings.find(category); if (wi != warnings.end()) { if (wi->second) { - this->CurrentSnapshot.PromoteDiagnostic( // clang-format: break - category, cmDiagnostics::Warn, true); + this->AlterDiagnostic(&cmStateSnapshot::PromoteDiagnostic, category, + cmDiagnostics::Warn, true); } else { - this->CurrentSnapshot.DemoteDiagnostic( // clang-format: break - category, cmDiagnostics::Ignore, true); + this->AlterDiagnostic(&cmStateSnapshot::DemoteDiagnostic, category, + cmDiagnostics::Ignore, true); } } auto const ei = errors.find(category); if (ei != errors.end()) { if (ei->second) { - this->CurrentSnapshot.PromoteDiagnostic( // clang-format: break - category, cmDiagnostics::SendError, true); + this->AlterDiagnostic(&cmStateSnapshot::PromoteDiagnostic, category, + cmDiagnostics::SendError, true); } else { - this->CurrentSnapshot.DemoteDiagnostic( // clang-format: break - category, cmDiagnostics::Warn, true); + this->AlterDiagnostic(&cmStateSnapshot::DemoteDiagnostic, category, + cmDiagnostics::Warn, true); } } } @@ -676,13 +669,13 @@ bool cmake::SetCacheArgs(std::vector const& args) } if (foundNo) { - state->CurrentSnapshot.DemoteDiagnostic( - *category, foundError ? cmDiagnostics::Warn : cmDiagnostics::Ignore, - true); + state->AlterDiagnostic( + &cmStateSnapshot::DemoteDiagnostic, *category, + foundError ? cmDiagnostics::Warn : cmDiagnostics::Ignore, true); } else { - state->CurrentSnapshot.PromoteDiagnostic( - *category, foundError ? cmDiagnostics::SendError : cmDiagnostics::Warn, - true); + state->AlterDiagnostic( + &cmStateSnapshot::PromoteDiagnostic, *category, + foundError ? cmDiagnostics::SendError : cmDiagnostics::Warn, true); } return true; }; @@ -1333,8 +1326,9 @@ void cmake::SetArgs(std::vector const& args) "--warn-uninitialized", CommandArgument::Values::Zero, [](std::string const&, cmake* state) -> bool { warnDeprecated("--warn-uninitialized"_s, "-Wuninitialized"_s); - state->CurrentSnapshot.PromoteDiagnostic( - cmDiagnostics::CMD_UNINITIALIZED, cmDiagnostics::Warn, true); + state->AlterDiagnostic(&cmStateSnapshot::PromoteDiagnostic, + cmDiagnostics::CMD_UNINITIALIZED, + cmDiagnostics::Warn, true); return true; } }, CommandArgument{ "--warn-unused-vars", CommandArgument::Values::Zero, @@ -1343,8 +1337,9 @@ void cmake::SetArgs(std::vector const& args) "--no-warn-unused-cli", CommandArgument::Values::Zero, [](std::string const&, cmake* state) -> bool { warnDeprecated("--no-warn-unused-cli"_s, "-Wno-unused-cli"_s); - state->CurrentSnapshot.DemoteDiagnostic(cmDiagnostics::CMD_UNUSED_CLI, - cmDiagnostics::Ignore, true); + state->AlterDiagnostic(&cmStateSnapshot::DemoteDiagnostic, + cmDiagnostics::CMD_UNUSED_CLI, + cmDiagnostics::Ignore, true); return true; } }, CommandArgument{ @@ -2450,13 +2445,9 @@ int cmake::Configure() #endif // We now need to harmonize the previous initial diagnostic state with any - // changes requested via command line options. This is a bit tricky, because - // we need to underlay what is specified by the cache beneath whatever state - // has been built from command line processing. - - cmDiagnosticAction deprecated = this->CurrentSnapshot.GetDiagnostic( - cmDiagnostics::CMD_DEPRECATED, cmDiagnostics::Undefined); - bool const deprecatedAlreadySet = (deprecated != cmDiagnostics::Undefined); + // changes requested via command line options and/or presets. We do this by + // first applying the prior (cached) state, then applying all deferred + // alterations. if (cmValue cachedDiagnostics = this->State->GetCacheEntryValue("CMAKE_DIAGNOSTIC_INIT")) { @@ -2470,11 +2461,7 @@ int cmake::Configure() cmDiagnostics::GetDiagnosticAction(v.substr(n + 1)); if (category && action) { - // Only use the cache if command-line options have not modified the - // diagnostic. - if (isDiagnosticSet(this->CurrentSnapshot, *category)) { - this->CurrentSnapshot.SetDiagnostic(*category, *action, false); - } + this->CurrentSnapshot.SetDiagnostic(*category, *action, false); } } } @@ -2484,9 +2471,11 @@ int cmake::Configure() this->State->GetCacheEntryValue("CMAKE_WARN_DEPRECATED"); if (cachedWarnDeprecated) { if (cachedWarnDeprecated.IsOn()) { - deprecated = cmDiagnostics::Warn; + this->CurrentSnapshot.PromoteDiagnostic(cmDiagnostics::CMD_DEPRECATED, + cmDiagnostics::Warn, false); } else { - deprecated = cmDiagnostics::Ignore; + this->CurrentSnapshot.DemoteDiagnostic(cmDiagnostics::CMD_DEPRECATED, + cmDiagnostics::Ignore, false); } } @@ -2494,15 +2483,17 @@ int cmake::Configure() this->State->GetCacheEntryValue("CMAKE_ERROR_DEPRECATED"); if (cachedErrorDeprecated) { if (cachedErrorDeprecated.IsOn()) { - deprecated = cmDiagnostics::SendError; + this->CurrentSnapshot.PromoteDiagnostic(cmDiagnostics::CMD_DEPRECATED, + cmDiagnostics::SendError, false); + } else { + this->CurrentSnapshot.DemoteDiagnostic(cmDiagnostics::CMD_DEPRECATED, + cmDiagnostics::Warn, false); } } - if (!deprecatedAlreadySet && deprecated != cmDiagnostics::Undefined) { - // CMD_DEPRECATED was not set by command-line options, but was altered by - // one or both of CMAKE_{WARN,ERROR}_DEPRECATED. - this->CurrentSnapshot.SetDiagnostic(cmDiagnostics::CMD_DEPRECATED, - deprecated, false); + for (DiagnosticAlteration const& da : this->DiagnosticAlterations) { + (this->CurrentSnapshot.*da.Alteration)(da.Category, da.DesiredAction, + da.Recurse); } // Now write the diagnostic state back to the cache. @@ -4508,6 +4499,21 @@ void cmake::RunCheckForUnusedVariables() #endif } +void cmake::AlterDiagnostic(DiagnosticAlterationMethod alteration, + cmDiagnosticCategory category, + cmDiagnosticAction desiredAction, bool recurse) +{ + // In Project mode, we need to defer applying any diagnostic changes until + // after reading the prior state from the cache. In all other modes, changes + // should be applied immediately. + if (this->State->GetRole() == cmState::Role::Project) { + this->DiagnosticAlterations.emplace_back( // clang-format: break + DiagnosticAlteration{ alteration, category, desiredAction, recurse }); + } else { + (this->CurrentSnapshot.*alteration)(category, desiredAction, recurse); + } +} + void cmake::SetDebugFindOutputPkgs(std::string const& args) { this->DebugFindPkgs.emplace(args); diff --git a/Source/cmake.h b/Source/cmake.h index 311c858674..cf9aacf01d 100644 --- a/Source/cmake.h +++ b/Source/cmake.h @@ -820,6 +820,23 @@ private: cmStateSnapshot CurrentSnapshot; std::unique_ptr Messenger; + using DiagnosticAlterationMethod = + void (cmStateSnapshot::*)(cmDiagnosticCategory, cmDiagnosticAction, bool); + + struct DiagnosticAlteration + { + DiagnosticAlterationMethod const Alteration; + cmDiagnosticCategory const Category; + cmDiagnosticAction const DesiredAction; + bool const Recurse; + }; + + std::vector DiagnosticAlterations; + + void AlterDiagnostic(DiagnosticAlterationMethod alteration, + cmDiagnosticCategory category, + cmDiagnosticAction desiredAction, bool recurse); + #ifndef CMAKE_BOOTSTRAP bool SarifFileOutput = false; std::string SarifFilePath; diff --git a/Tests/RunCMake/Diagnostics/Persist1.cmake b/Tests/RunCMake/Diagnostics/Persist1.cmake new file mode 100644 index 0000000000..46c7215d57 --- /dev/null +++ b/Tests/RunCMake/Diagnostics/Persist1.cmake @@ -0,0 +1,4 @@ +include(Assertions.cmake) + +expect_cached(CMD_AUTHOR IGNORE) +expect_cached(CMD_DEPRECATED IGNORE) diff --git a/Tests/RunCMake/Diagnostics/Persist2.cmake b/Tests/RunCMake/Diagnostics/Persist2.cmake new file mode 100644 index 0000000000..161d5ed8b5 --- /dev/null +++ b/Tests/RunCMake/Diagnostics/Persist2.cmake @@ -0,0 +1,4 @@ +include(Assertions.cmake) + +expect_cached(CMD_AUTHOR WARN) +expect_cached(CMD_DEPRECATED WARN) diff --git a/Tests/RunCMake/Diagnostics/RunCMakeTest.cmake b/Tests/RunCMake/Diagnostics/RunCMakeTest.cmake index 924ef61f15..630d18229b 100644 --- a/Tests/RunCMake/Diagnostics/RunCMakeTest.cmake +++ b/Tests/RunCMake/Diagnostics/RunCMakeTest.cmake @@ -14,3 +14,18 @@ run_cmake_with_options(CommandLine1 -Werror=author -Wdeprecated) run_cmake_with_options(CommandLine1 -Wno-deprecated -Werror=author) run_cmake_with_options(CommandLine2 -Werror=author -Wno-deprecated) run_cmake_with_options(CommandLine3 -Wno-error=uninitialized -Winstall-absolute-destination) + +function(run_persist_test NAME) + set(RunCMake_TEST_VARIANT_DESCRIPTION "-first") + run_cmake_with_options(${NAME} ${ARGN}) + set(RunCMake_TEST_NO_CLEAN 1) + set(RunCMake_TEST_VARIANT_DESCRIPTION "-second") + run_cmake(${NAME}) +endfunction() + +block() + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/Persist-build) + run_persist_test(Persist1 -Wno-author) + set(RunCMake_TEST_NO_CLEAN 1) + run_persist_test(Persist2 -Wauthor) +endblock()