release/21.x: [libc++][hardening] Introduce assertion semantics. (#149459) #151095
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
@var-const What do you think about merging this PR to the release branch? |
llvmbot
added
libc++
|
@llvm/pr-subscribers-libcxxabi @llvm/pr-subscribers-github-workflow Author: None (llvmbot) ChangesBackport 3eee9fc Requested by: @var-const Patch is 31.22 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/151095.diff 15 Files Affected:
diff --git a/.github/workflows/libcxx-build-and-test.yaml b/.github/workflows/libcxx-build-and-test.yaml
index a5be912b1ab07..7c18560bd163a 100644
--- a/.github/workflows/libcxx-build-and-test.yaml
+++ b/.github/workflows/libcxx-build-and-test.yaml
@@ -128,6 +128,7 @@ jobs:
'generic-abi-unstable',
'generic-hardening-mode-debug',
'generic-hardening-mode-extensive',
+ 'generic-hardening-mode-extensive-observe-semantic',
'generic-hardening-mode-fast',
'generic-hardening-mode-fast-with-abi-breaks',
'generic-merged',
diff --git a/libcxx/cmake/caches/Generic-hardening-mode-extensive-observe-semantic.cmake b/libcxx/cmake/caches/Generic-hardening-mode-extensive-observe-semantic.cmake
new file mode 100644
index 0000000000000..c843c02977a87
--- /dev/null
+++ b/libcxx/cmake/caches/Generic-hardening-mode-extensive-observe-semantic.cmake
@@ -0,0 +1,2 @@
+set(LIBCXX_HARDENING_MODE "extensive" CACHE STRING "")
+set(LIBCXX_TEST_PARAMS "assertion_semantic=observe" CACHE STRING "")
diff --git a/libcxx/docs/Hardening.rst b/libcxx/docs/Hardening.rst
index 17808841bd9ec..1cdb3605c38ab 100644
--- a/libcxx/docs/Hardening.rst
+++ b/libcxx/docs/Hardening.rst
@@ -39,6 +39,8 @@ modes are:
Enabling hardening has no impact on the ABI.
+.. _notes-for-users:
+
Notes for users
---------------
@@ -72,6 +74,10 @@ to control the level by passing **one** of the following options to the compiler
pre-built components. Most libc++ code is header-based, so a user-provided
value for ``_LIBCPP_HARDENING_MODE`` will be mostly respected.
+In some cases, users might want to override the assertion semantic used by the
+library. This can be done similarly to setting the hardening mode; please refer
+to the :ref:`relevant section <assertion-semantics>`.
+
Notes for vendors
-----------------
@@ -260,6 +266,68 @@ output. This is less secure and increases the size of the binary (among other
things, it has to store the error message strings) but makes the failure easier
to debug. It also allows testing the error messages in our test suite.
+This default behavior can be customized by users via :ref:`assertion semantics
+<assertion-semantics>`; it can also be completely overridden by vendors by
+providing a :ref:`custom assertion failure handler
+<override-assertion-handler>`.
+
+.. _assertion-semantics:
+
+Assertion semantics
+-------------------
+
+.. warning::
+
+ Assertion semantics are currently an experimental feature.
+
+.. note::
+
+ Assertion semantics are not available in the C++03 mode.
+
+What happens when an assertion fails depends on the assertion semantic being
+used. Four assertion semantics are available, based on C++26 Contracts
+evaluation semantics:
+
+- ``ignore`` evaluates the assertion but has no effect if it fails (note that it
+ differs from the Contracts ``ignore`` semantic which would not evaluate
+ the assertion at all);
+- ``observe`` logs an error (indicating, if possible on the platform, that the
+ error is fatal) but continues execution;
+- ``quick-enforce`` terminates the program as fast as possible via a trap
+ instruction. It is the default semantic for the production modes (``fast`` and
+ ``extensive``);
+- ``enforce`` logs an error and then terminates the program. It is the default
+ semantic for the ``debug`` mode.
+
+Notes:
+
+- Continuing execution after a hardening check fails results in undefined
+ behavior; the ``observe`` semantic is meant to make adopting hardening easier
+ but should not be used outside of the adoption period;
+- C++26 wording for Library Hardening precludes a conforming Hardened
+ implementation from using the Contracts ``ignore`` semantic when evaluating
+ hardened preconditions in the Library. Libc++ allows using this semantic for
+ hardened preconditions, but please be aware that using ``ignore`` does not
+ produce a conforming "Hardened" implementation, unlike the other semantics
+ above.
+
+The default assertion semantics are as follows:
+
+- ``fast``: ``quick-enforce``;
+- ``extensive``: ``quick-enforce``;
+- ``debug``: ``enforce``.
+
+The default assertion semantics can be overridden by passing **one** of the
+following options to the compiler:
+
+- ``-D_LIBCPP_ASSERTION_SEMANTIC=_LIBCPP_ASSERTION_SEMANTIC_IGNORE``
+- ``-D_LIBCPP_ASSERTION_SEMANTIC=_LIBCPP_ASSERTION_SEMANTIC_OBSERVE``
+- ``-D_LIBCPP_ASSERTION_SEMANTIC=_LIBCPP_ASSERTION_SEMANTIC_QUICK_ENFORCE``
+- ``-D_LIBCPP_ASSERTION_SEMANTIC=_LIBCPP_ASSERTION_SEMANTIC_ENFORCE``
+
+All the :ref:`same notes <notes-for-users>` apply to setting this macro as for
+setting ``_LIBCPP_HARDENING_MODE``.
+
.. _override-assertion-handler:
Overriding the assertion failure handler
diff --git a/libcxx/docs/ReleaseNotes/21.rst b/libcxx/docs/ReleaseNotes/21.rst
index d31ca0130cb80..74bfa97fd21c2 100644
--- a/libcxx/docs/ReleaseNotes/21.rst
+++ b/libcxx/docs/ReleaseNotes/21.rst
@@ -88,6 +88,12 @@ Improvements and New Features
- ``ctype::tolower`` and ``ctype::toupper`` have been optimized, resulting in a 2x performance improvement.
+- As an experimental feature, Hardening now supports assertion semantics that allow customizing how a hardening
+ assertion failure is handled. The four available semantics, modeled on C++26 Contracts, are ``ignore``, ``observe``,
+ ``quick-enforce`` and ``enforce``. The ``observe`` semantic is intended to make it easier to adopt Hardening in
+ production but should not be used outside of this scenario. Please refer to the :ref:`Hardening documentation
+ <hardening>` for details.
+
Deprecations and Removals
-------------------------
diff --git a/libcxx/docs/UserDocumentation.rst b/libcxx/docs/UserDocumentation.rst
index 79f5908bdc6b6..415a599168374 100644
--- a/libcxx/docs/UserDocumentation.rst
+++ b/libcxx/docs/UserDocumentation.rst
@@ -72,6 +72,11 @@ when ``-fexperimental-library`` is passed:
* ``std::chrono::tzdb`` and related time zone functionality
* ``<syncstream>``
+Additionally, assertion semantics are an experimental feature that can be used
+to customize the behavior of Hardening (see :ref:`here <assertion-semantics>`).
+Assertion semantics mirror the evaluation semantics of C++26 Contracts but are
+not a standard feature.
+
.. note::
Experimental libraries are experimental.
* The contents of the ``<experimental/...>`` headers and the associated static
diff --git a/libcxx/include/__config b/libcxx/include/__config
index 7bb5d9ac77ea4..3f41e044862ec 100644
--- a/libcxx/include/__config
+++ b/libcxx/include/__config
@@ -38,6 +38,30 @@
# define _LIBCPP_FREESTANDING
# endif
+// NOLINTNEXTLINE(libcpp-cpp-version-check)
+# if __cplusplus < 201103L
+# define _LIBCPP_CXX03_LANG
+# endif
+
+# if __has_feature(experimental_library)
+# ifndef _LIBCPP_ENABLE_EXPERIMENTAL
+# define _LIBCPP_ENABLE_EXPERIMENTAL
+# endif
+# endif
+
+// Incomplete features get their own specific disabling flags. This makes it
+// easier to grep for target specific flags once the feature is complete.
+# if defined(_LIBCPP_ENABLE_EXPERIMENTAL) || defined(_LIBCPP_BUILDING_LIBRARY)
+# define _LIBCPP_HAS_EXPERIMENTAL_LIBRARY 1
+# else
+# define _LIBCPP_HAS_EXPERIMENTAL_LIBRARY 0
+# endif
+
+# define _LIBCPP_HAS_EXPERIMENTAL_PSTL _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
+# define _LIBCPP_HAS_EXPERIMENTAL_TZDB _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
+# define _LIBCPP_HAS_EXPERIMENTAL_SYNCSTREAM _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
+# define _LIBCPP_HAS_EXPERIMENTAL_HARDENING_OBSERVE_SEMANTIC _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
+
// HARDENING {
// TODO: Remove in LLVM 21. We're making this an error to catch folks who might not have migrated.
@@ -147,16 +171,53 @@ _LIBCPP_HARDENING_MODE_EXTENSIVE, \
_LIBCPP_HARDENING_MODE_DEBUG
# endif
+// Hardening assertion semantics generally mirror the evaluation semantics of C++26 Contracts:
+// - `ignore` evaluates the assertion but doesn't do anything if it fails (note that it differs from the Contracts
+// `ignore` semantic which wouldn't evaluate the assertion at all);
+// - `observe` logs an error (indicating, if possible, that the error is fatal) and continues execution;
+// - `quick-enforce` terminates the program as fast as possible (via trapping);
+// - `enforce` logs an error and then terminates the program.
+//
+// Notes:
+// - Continuing execution after a hardening check fails results in undefined behavior; the `observe` semantic is meant
+// to make adopting hardening easier but should not be used outside of this scenario;
+// - C++26 wording for Library Hardening precludes a conforming Hardened implementation from using the Contracts
+// `ignore` semantic when evaluating hardened preconditions in the Library. Libc++ allows using this semantic for
+// hardened preconditions, however, be aware that using `ignore` does not produce a conforming "Hardened"
+// implementation, unlike the other semantics above.
+// clang-format off
+# define _LIBCPP_ASSERTION_SEMANTIC_IGNORE (1 << 1)
+# define _LIBCPP_ASSERTION_SEMANTIC_OBSERVE (1 << 2)
+# define _LIBCPP_ASSERTION_SEMANTIC_QUICK_ENFORCE (1 << 3)
+# define _LIBCPP_ASSERTION_SEMANTIC_ENFORCE (1 << 4)
+// clang-format on
+
+// Allow users to define an arbitrary assertion semantic; otherwise, use the default mapping from modes to semantics.
+// The default is for production-capable modes to use `quick-enforce` (i.e., trap) and for the `debug` mode to use
+// `enforce` (i.e., log and abort).
+# ifndef _LIBCPP_ASSERTION_SEMANTIC
+
+# if _LIBCPP_HARDENING_MODE == _LIBCPP_HARDENING_MODE_DEBUG
+# define _LIBCPP_ASSERTION_SEMANTIC _LIBCPP_ASSERTION_SEMANTIC_ENFORCE
+# else
+# define _LIBCPP_ASSERTION_SEMANTIC _LIBCPP_ASSERTION_SEMANTIC_QUICK_ENFORCE
+# endif
+
+# else
+# if !_LIBCPP_HAS_EXPERIMENTAL_LIBRARY
+# error "Assertion semantics are an experimental feature."
+# endif
+# if defined(_LIBCPP_CXX03_LANG)
+# error "Assertion semantics are not available in the C++03 mode."
+# endif
+
+# endif // _LIBCPP_ASSERTION_SEMANTIC
+
// } HARDENING
# define _LIBCPP_TOSTRING2(x) #x
# define _LIBCPP_TOSTRING(x) _LIBCPP_TOSTRING2(x)
-// NOLINTNEXTLINE(libcpp-cpp-version-check)
-# if __cplusplus < 201103L
-# define _LIBCPP_CXX03_LANG
-# endif
-
# ifndef __has_constexpr_builtin
# define __has_constexpr_builtin(x) 0
# endif
@@ -190,25 +251,6 @@ _LIBCPP_HARDENING_MODE_DEBUG
# define _LIBCPP_ABI_VCRUNTIME
# endif
-# if __has_feature(experimental_library)
-# ifndef _LIBCPP_ENABLE_EXPERIMENTAL
-# define _LIBCPP_ENABLE_EXPERIMENTAL
-# endif
-# endif
-
-// Incomplete features get their own specific disabling flags. This makes it
-// easier to grep for target specific flags once the feature is complete.
-# if defined(_LIBCPP_ENABLE_EXPERIMENTAL) || defined(_LIBCPP_BUILDING_LIBRARY)
-# define _LIBCPP_HAS_EXPERIMENTAL_LIBRARY 1
-# else
-# define _LIBCPP_HAS_EXPERIMENTAL_LIBRARY 0
-# endif
-
-# define _LIBCPP_HAS_EXPERIMENTAL_PSTL _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
-# define _LIBCPP_HAS_EXPERIMENTAL_TZDB _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
-# define _LIBCPP_HAS_EXPERIMENTAL_SYNCSTREAM _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
-# define _LIBCPP_HAS_EXPERIMENTAL_HARDENING_OBSERVE_SEMANTIC _LIBCPP_HAS_EXPERIMENTAL_LIBRARY
-
# if defined(__MVS__)
# include <features.h> // for __NATIVE_ASCII_F
# endif
diff --git a/libcxx/include/__cxx03/__config b/libcxx/include/__cxx03/__config
index ef47327d96355..9b88a495055ee 100644
--- a/libcxx/include/__cxx03/__config
+++ b/libcxx/include/__cxx03/__config
@@ -152,6 +152,10 @@ _LIBCPP_HARDENING_MODE_EXTENSIVE, \
_LIBCPP_HARDENING_MODE_DEBUG
# endif
+# ifdef _LIBCPP_ASSERTION_SEMANTIC
+# error "Assertion semantics are not available in the C++03 mode."
+# endif
+
// } HARDENING
# define _LIBCPP_TOSTRING2(x) #x
diff --git a/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp b/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp
index 419a603a037f8..2bc4648878f8e 100644
--- a/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp
+++ b/libcxx/test/libcxx/thread/thread.barrier/assert.arrive.pass.cpp
@@ -8,6 +8,8 @@
// UNSUPPORTED: no-threads
// UNSUPPORTED: c++03, c++11, c++14, c++17
// REQUIRES: libcpp-hardening-mode={{extensive|debug}}
+// Without the assertion, the test will most likely time out.
+// UNSUPPORTED: libcpp-assertion-semantic={{ignore|observe}}
// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
diff --git a/libcxx/test/libcxx/thread/thread.latch/assert.arrive_and_wait.pass.cpp b/libcxx/test/libcxx/thread/thread.latch/assert.arrive_and_wait.pass.cpp
index e61679554a62e..30d36b5f6d7b5 100644
--- a/libcxx/test/libcxx/thread/thread.latch/assert.arrive_and_wait.pass.cpp
+++ b/libcxx/test/libcxx/thread/thread.latch/assert.arrive_and_wait.pass.cpp
@@ -18,6 +18,8 @@
// REQUIRES: has-unix-headers
// REQUIRES: libcpp-hardening-mode={{extensive|debug}}
+// Without the assertion, the test will most likely time out.
+// UNSUPPORTED: libcpp-assertion-semantic={{ignore|observe}}
// XFAIL: libcpp-hardening-mode=debug && availability-verbose_abort-missing
#include <latch>
diff --git a/libcxx/test/support/check_assertion.h b/libcxx/test/support/check_assertion.h
index a279400d651b4..a091043195345 100644
--- a/libcxx/test/support/check_assertion.h
+++ b/libcxx/test/support/check_assertion.h
@@ -44,15 +44,32 @@ static constexpr const char* Marker = "###";
using MatchResult = std::pair<bool, std::string>;
using Matcher = std::function<MatchResult(const std::string& /*text*/)>;
-MatchResult MatchAssertionMessage(const std::string& text, std::string_view expected_message) {
+// Using the marker makes matching more precise, but we cannot output the marker when the `observe` semantic is used
+// (because it doesn't allow customizing the logging function). If the marker is not available, fall back to using less
+// precise matching by just the error message.
+MatchResult MatchAssertionMessage(const std::string& text, std::string_view expected_message, bool use_marker) {
// Extract information from the error message. This has to stay synchronized with how we format assertions in the
// library.
- std::regex assertion_format(".*###\\n(.*):(\\d+): assertion (.*) failed: (.*)\\n###");
+ std::string assertion_format_string = [&] {
+ if (use_marker)
+ return (".*###\\n(.*):(\\d+): assertion (.*) failed: (.*)\\n###");
+ return ("(.*):(\\d+): assertion (.*) failed: (.*)\\n");
+ }();
+ std::regex assertion_format(assertion_format_string);
std::smatch match_result;
- bool has_match = std::regex_match(text, match_result, assertion_format);
- assert(has_match);
- assert(match_result.size() == 5);
+ // If a non-terminating assertion semantic is used, more than one assertion might be triggered before the process
+ // dies, so we cannot expect the entire target string to match.
+ bool has_match = std::regex_search(text, match_result, assertion_format);
+ if (!has_match || match_result.size() != 5) {
+ std::stringstream matching_error;
+ matching_error //
+ << "Failed to parse the assertion message.\n" //
+ << "Using marker: " << use_marker << "\n" //
+ << "Expected message: '" << expected_message.data() << "'\n" //
+ << "Stderr contents: '" << text.c_str() << "'\n";
+ return MatchResult(/*success=*/false, matching_error.str());
+ }
const std::string& file = match_result[1];
int line = std::stoi(match_result[2]);
@@ -72,9 +89,9 @@ MatchResult MatchAssertionMessage(const std::string& text, std::string_view expe
return MatchResult(/*success=*/true, /*maybe_error=*/"");
}
-Matcher MakeAssertionMessageMatcher(std::string_view assertion_message) {
+Matcher MakeAssertionMessageMatcher(std::string_view assertion_message, bool use_marker = true) {
return [=](const std::string& text) { //
- return MatchAssertionMessage(text, assertion_message);
+ return MatchAssertionMessage(text, assertion_message, use_marker);
};
}
@@ -85,13 +102,17 @@ Matcher MakeAnyMatcher() {
}
enum class DeathCause {
- // Valid causes
+ // Valid causes.
VerboseAbort = 1,
StdAbort,
StdTerminate,
Trap,
- // Invalid causes
+ // Causes that might be invalid or might stem from undefined behavior (relevant for non-terminating assertion
+ // semantics).
DidNotDie,
+ Segfault,
+ ArithmeticError,
+ // Always invalid causes.
SetupFailure,
Unknown
};
@@ -108,6 +129,16 @@ bool IsValidCause(DeathCause cause) {
}
}
+bool IsTestSetupErrorCause(DeathCause cause) {
+ switch (cause) {
+ case DeathCause::SetupFailure:
+ case DeathCause::Unknown:
+ return true;
+ default:
+ return false;
+ }
+}
+
std::string ToString(DeathCause cause) {
switch (cause) {
case DeathCause::VerboseAbort:
@@ -120,10 +151,14 @@ std::string ToString(DeathCause cause) {
return "trap";
case DeathCause::DidNotDie:
return "<invalid cause (child did not die)>";
+ case DeathCause::Segfault:
+ return "<invalid cause (segmentation fault)>";
+ case DeathCause::ArithmeticError:
+ return "<invalid cause (fatal arithmetic error)>";
case DeathCause::SetupFailure:
- return "<invalid cause (child failed to set up test environment)>";
+ return "<test setup error (child failed to set up test environment)>";
case DeathCause::Unknown:
- return "<invalid cause (cause unknown)>";
+ return "<test setup error (test doesn't know how to interpret the death cause)>";
}
assert(false && "Unreachable");
@@ -225,9 +260,38 @@ class DeathTest {
return DeathTestResult(Outcome::Success, cause);
}
- void PrintFailureDetails(std::string_view failure_description, std::string_view stmt, DeathCause cause) const {
- std::fprintf(
- stderr, "Failure: EXPECT_DEATH( %s ) failed!\n(reason: %s)\n\n", stmt.data(), failure_description.data());
+ // When non-terminating assertion semantics are used, the program will invoke UB which might or might not crash the
+ // process; we make sure that the execution produces the expected error message but otherwise consider the test run
+ // successful whether the child process dies or not.
+ template <class Func>
+ DeathTestResult RunWithoutGuaranteedDeath(Func&& func, const Matcher& matcher) {
+ std::signal(SIGABRT, [](int) { StopChildProcess(DeathCause::StdAbort); });
+ std::set_terminate([] { StopChildProcess(DeathCause::StdTerminate); });
+
+ DeathCause cause = Run(func);
+
+ if (IsTestSetupErrorCause(cause)) {
+ return DeathTestResult(Outcome::InvalidCause, cause, ToString(cause));
+ }
+
+ MatchResult match_result = matcher(GetChildStdErr());
+ if (!match_result.first) {
+ auto failure_description = std::string("Child produced a different error message\n") + match_result.second;
+ return DeathTestResult(Outcome::UnexpectedErrorMessage, cause, failure_description);
+ }
+
+ return DeathTestResult(Outcome::Success, cause);
+ }
+
+ void PrintFailureDetails(std::string_view invocation,
+ std::string_view failure_description,
+ std::string_view stmt,
+ DeathCause cause) const {
+ std::fprintf(stderr,
+ "Failure: %s( %s ) failed!\n(reason: %s)\n\n",
+ invocation.data(),
+ stmt.data(),
+ failure_description.data());
if (cause != DeathCause::Unknown) {
std::fprintf(stderr, "child exit code: %d\n", GetChildExitCode());
@@ -311,10 +375,16 @@ class DeathTest {
if (WIFSIGNALED(status_value)) {
exit_code_ = WTERMSIG(status_value);
- // `__builtin_trap` generqtes `SIGILL` on x86 and `SIGTRAP` on ARM.
+ // `__builtin_trap` generates `SIGILL` on x86 and `SIGTRAP` on ARM.
if (exit_code_ == SIGILL || exit_code_ == SIGTRAP) {
return DeathCause::Trap;
}
+ if (exit_code_ == SIGSEGV) {
+ return DeathCause::Segfault;
+ }
+ if (exit_code_ == SIGFPE) {
+ return DeathCause::ArithmeticError;
+ }
}
return DeathCause::Unknown;
@@ -357,7 +427,7 @@ bool ExpectDeath(
DeathTest test_case;
DeathTestResult test_result = test_case.Run(expected_causes, func, matcher)...
[truncated]
|
|
Reason for cherry-picking: this patch is a follow-up to #150481 and the same rationale applies: we want to gain real-world experience with this under the experimental flag to allow us to roll this out in the next release. |
var-const
approved these changes
Jul 29, 2025
github-project-automation
bot
moved this from Needs Triage
to Needs Merge
in LLVM Release Status
|
@ldionne Are you happy with us merging this? I am guessing yes - but I want to double check :) |
|
@tru Friendly ping. :) Sorry for the nuisance -- I just want to make sure this patch doesn't miss the current RC deadline. |
|
Also cc @tstellar (sorry, not sure who is the most appropriate person to ping, apologies for the noise). |
|
There is no stress. I will merge it when I merge patches the next time. |
Assertion semantics closely mimic C++26 Contracts evaluation semantics. This brings our implementation closer in line with C++26 Library Hardening (one particular benefit is that using the `observe` semantic makes adopting hardening easier for projects). (cherry picked from commit 3eee9fc)
github-project-automation
bot
moved this from Needs Merge
to Done
in LLVM Release Status
|
@var-const (or anyone else). If you would like to add a note about this fix in the release notes (completely optional). Please reply to this comment with a one or two sentence description of the fix. When you are done, please add the release:note label to this PR. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Backport 3eee9fc
Requested by: @var-const