[lldb] Make deep copies of Status explicit (NFC) #107170
Merged
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
adrian-prantl
requested review from
bulbazord and
medismailben
and removed request for
JDevlieghere
|
@llvm/pr-subscribers-lldb Author: Adrian Prantl (adrian-prantl) ChangesBroken out from #106774 for easier review. Patch is 108.56 KiB, truncated to 20.00 KiB below, full version: https://github.com/llvm/llvm-project/pull/107170.diff 91 Files Affected:
diff --git a/lldb/bindings/python/python-swigsafecast.swig b/lldb/bindings/python/python-swigsafecast.swig
index 34f8c6f0ff8d35..bffd10ae7b3315 100644
--- a/lldb/bindings/python/python-swigsafecast.swig
+++ b/lldb/bindings/python/python-swigsafecast.swig
@@ -34,7 +34,7 @@ PythonObject SWIGBridge::ToSWIGWrapper(lldb::BreakpointSP breakpoint_sp) {
}
PythonObject SWIGBridge::ToSWIGWrapper(const Status& status) {
- return ToSWIGHelper(new lldb::SBError(status), SWIGTYPE_p_lldb__SBError);
+ return ToSWIGHelper(new lldb::SBError(status.Clone()), SWIGTYPE_p_lldb__SBError);
}
PythonObject SWIGBridge::ToSWIGWrapper(std::unique_ptr<lldb::SBStructuredData> data_sb) {
diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h
index 17f2c6c3027af7..9f55f92131c06e 100644
--- a/lldb/include/lldb/API/SBError.h
+++ b/lldb/include/lldb/API/SBError.h
@@ -97,7 +97,7 @@ class LLDB_API SBError {
friend class lldb_private::ScriptInterpreter;
friend class lldb_private::python::SWIGBridge;
- SBError(const lldb_private::Status &error);
+ SBError(lldb_private::Status &&error);
lldb_private::Status *get();
@@ -107,7 +107,7 @@ class LLDB_API SBError {
lldb_private::Status &ref();
- void SetError(const lldb_private::Status &lldb_error);
+ void SetError(lldb_private::Status &&lldb_error);
private:
std::unique_ptr<lldb_private::Status> m_opaque_up;
diff --git a/lldb/include/lldb/API/SBValueList.h b/lldb/include/lldb/API/SBValueList.h
index a5017bccc50533..52a86f989e153a 100644
--- a/lldb/include/lldb/API/SBValueList.h
+++ b/lldb/include/lldb/API/SBValueList.h
@@ -96,7 +96,7 @@ class LLDB_API SBValueList {
std::unique_ptr<ValueListImpl> m_opaque_up;
- void SetError(const lldb_private::Status &status);
+ void SetError(lldb_private::Status &&status);
};
} // namespace lldb
diff --git a/lldb/include/lldb/Core/ValueObjectConstResult.h b/lldb/include/lldb/Core/ValueObjectConstResult.h
index d3b3362bd0e9ec..9c34617af71d0d 100644
--- a/lldb/include/lldb/Core/ValueObjectConstResult.h
+++ b/lldb/include/lldb/Core/ValueObjectConstResult.h
@@ -61,7 +61,7 @@ class ValueObjectConstResult : public ValueObject {
// When an expression fails to evaluate, we return an error
static lldb::ValueObjectSP Create(ExecutionContextScope *exe_scope,
- const Status &error);
+ Status &&error);
std::optional<uint64_t> GetByteSize() override;
@@ -146,7 +146,7 @@ class ValueObjectConstResult : public ValueObject {
ConstString name, Module *module = nullptr);
ValueObjectConstResult(ExecutionContextScope *exe_scope,
- ValueObjectManager &manager, const Status &error);
+ ValueObjectManager &manager, Status &&error);
ValueObject *CreateChildAtIndex(size_t idx) override {
return m_impl.CreateChildAtIndex(idx);
diff --git a/lldb/include/lldb/Target/Process.h b/lldb/include/lldb/Target/Process.h
index a7de991104434d..c66cfb2c245efd 100644
--- a/lldb/include/lldb/Target/Process.h
+++ b/lldb/include/lldb/Target/Process.h
@@ -1296,8 +1296,6 @@ class Process : public std::enable_shared_from_this<Process>,
const EvaluateExpressionOptions &options,
DiagnosticManager &diagnostic_manager);
- static const char *ExecutionResultAsCString(lldb::ExpressionResults result);
-
void GetStatus(Stream &ostrm);
size_t GetThreadStatus(Stream &ostrm, bool only_threads_with_stop_reason,
diff --git a/lldb/include/lldb/Utility/Status.h b/lldb/include/lldb/Utility/Status.h
index b304291ffae00e..c241a67d027c67 100644
--- a/lldb/include/lldb/Utility/Status.h
+++ b/lldb/include/lldb/Utility/Status.h
@@ -26,6 +26,36 @@ class raw_ostream;
namespace lldb_private {
+class MachKernelError
+ : public llvm::ErrorInfo<MachKernelError, llvm::StringError> {
+public:
+ using llvm::ErrorInfo<MachKernelError, llvm::StringError>::ErrorInfo;
+ MachKernelError(std::error_code ec, const llvm::Twine &msg = {})
+ : ErrorInfo(msg, ec) {}
+ std::string message() const override;
+ static char ID;
+};
+
+class Win32Error : public llvm::ErrorInfo<Win32Error, llvm::StringError> {
+public:
+ using llvm::ErrorInfo<Win32Error, llvm::StringError>::ErrorInfo;
+ Win32Error(std::error_code ec, const llvm::Twine &msg = {})
+ : ErrorInfo(msg, ec) {}
+ std::string message() const override;
+ static char ID;
+};
+
+class ExpressionError
+ : public llvm::ErrorInfo<ExpressionError, llvm::StringError> {
+public:
+ using llvm::ErrorInfo<ExpressionError, llvm::StringError>::ErrorInfo;
+ ExpressionError(std::error_code ec, std::string msg = {})
+ : ErrorInfo(msg, ec) {}
+ static char ID;
+};
+
+const char *ExecutionResultAsCString(lldb::ExpressionResults result);
+
/// \class Status Status.h "lldb/Utility/Status.h" An error handling class.
///
/// This class is designed to be able to hold any error code that can be
@@ -41,13 +71,32 @@ namespace lldb_private {
/// of themselves for printing results and error codes. The string value will
/// be fetched on demand and its string value will be cached until the error
/// is cleared of the value of the error changes.
+///
+/// API design notes:
+///
+/// Most APIs that currently vend a Status would be better served by
+/// returning llvm::Expected<> instead. If possibles APIs should be
+/// refactored to avoid Status. The only legitimate long-term uses of
+/// Status are objects that need to store an error for a long time
+/// (which should be questioned as a design decision, too).
+///
+/// Implementation notes:
+///
+/// Internally, Status stores an llvm::Error.
+/// eErrorTypeInvalid
+/// eErrorTypeGeneric llvm::StringError
+/// eErrorTypePOSIX llvm::ECError
+/// eErrorTypeMachKernel MachKernelError
+/// eErrorTypeExpression llvm::ErrorList<ExpressionError>
+/// eErrorTypeWin32 Win32Error
+
class Status {
public:
- /// Every error value that this object can contain needs to be able to fit
/// into ValueType.
typedef uint32_t ValueType;
Status();
+ Status(Status &&other) = default;
/// Initialize the error object with a generic success value.
///
@@ -91,10 +140,14 @@ class Status {
~Status();
- // llvm::Error support
- explicit Status(llvm::Error error) { *this = std::move(error); }
- const Status &operator=(llvm::Error error);
+ const Status &operator=(Status &&);
+ /// Avoid using this in new code. Migrate APIs to llvm::Expected instead.
+ static Status FromError(llvm::Error &&error);
+ /// FIXME: Replace this with a takeError() method.
llvm::Error ToError() const;
+ /// Don't call this function in new code. Instead, redesign the API
+ /// to use llvm::Expected instead of Status.
+ Status Clone() const { return Status(ToError()); }
/// Get the error string associated with the current error.
//
@@ -145,6 +198,7 @@ class Status {
bool Success() const;
protected:
+ Status(llvm::Error &&error);
/// Status code as an integer value.
ValueType m_code = 0;
/// The type of the above error code.
diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp
index 3e9c01080588e5..b2ed034d19983c 100644
--- a/lldb/source/API/SBBreakpoint.cpp
+++ b/lldb/source/API/SBBreakpoint.cpp
@@ -622,7 +622,7 @@ SBError SBBreakpoint::SetScriptCallbackFunction(
callback_function_name,
extra_args.m_impl_up
->GetObjectSP());
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
} else
sb_error = Status::FromErrorString("invalid breakpoint");
@@ -645,7 +645,7 @@ SBError SBBreakpoint::SetScriptCallbackBody(const char *callback_body_text) {
.GetScriptInterpreter()
->SetBreakpointCommandCallback(bp_options, callback_body_text,
/*is_callback=*/false);
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
} else
sb_error = Status::FromErrorString("invalid breakpoint");
@@ -670,7 +670,7 @@ SBError SBBreakpoint::AddNameWithErrorHandling(const char *new_name) {
bkpt_sp->GetTarget().GetAPIMutex());
Status error;
bkpt_sp->GetTarget().AddNameToBreakpoint(bkpt_sp, new_name, error);
- status.SetError(error);
+ status.SetError(std::move(error));
} else {
status = Status::FromErrorString("invalid breakpoint");
}
diff --git a/lldb/source/API/SBBreakpointLocation.cpp b/lldb/source/API/SBBreakpointLocation.cpp
index e5c96b81e80904..b2d1da3927c6ea 100644
--- a/lldb/source/API/SBBreakpointLocation.cpp
+++ b/lldb/source/API/SBBreakpointLocation.cpp
@@ -239,7 +239,7 @@ SBError SBBreakpointLocation::SetScriptCallbackFunction(
callback_function_name,
extra_args.m_impl_up
->GetObjectSP());
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
} else
sb_error = Status::FromErrorString("invalid breakpoint");
@@ -264,7 +264,7 @@ SBBreakpointLocation::SetScriptCallbackBody(const char *callback_body_text) {
.GetScriptInterpreter()
->SetBreakpointCommandCallback(bp_options, callback_body_text,
/*is_callback=*/false);
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
} else
sb_error = Status::FromErrorString("invalid breakpoint");
diff --git a/lldb/source/API/SBBreakpointName.cpp b/lldb/source/API/SBBreakpointName.cpp
index 7dc8dee19f43d2..831260d44e8e7f 100644
--- a/lldb/source/API/SBBreakpointName.cpp
+++ b/lldb/source/API/SBBreakpointName.cpp
@@ -570,14 +570,13 @@ SBError SBBreakpointName::SetScriptCallbackFunction(
m_impl_up->GetTarget()->GetAPIMutex());
BreakpointOptions &bp_options = bp_name->GetOptions();
- Status error;
- error = m_impl_up->GetTarget()
- ->GetDebugger()
- .GetScriptInterpreter()
- ->SetBreakpointCommandCallbackFunction(
- bp_options, callback_function_name,
- extra_args.m_impl_up->GetObjectSP());
- sb_error.SetError(error);
+ Status error = m_impl_up->GetTarget()
+ ->GetDebugger()
+ .GetScriptInterpreter()
+ ->SetBreakpointCommandCallbackFunction(
+ bp_options, callback_function_name,
+ extra_args.m_impl_up->GetObjectSP());
+ sb_error.SetError(std::move(error));
UpdateName(*bp_name);
return sb_error;
}
@@ -600,7 +599,7 @@ SBBreakpointName::SetScriptCallbackBody(const char *callback_body_text) {
.GetScriptInterpreter()
->SetBreakpointCommandCallback(
bp_options, callback_body_text, /*is_callback=*/false);
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
if (!sb_error.Fail())
UpdateName(*bp_name);
diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp
index 72501570320d57..b21d7e67290073 100644
--- a/lldb/source/API/SBDebugger.cpp
+++ b/lldb/source/API/SBDebugger.cpp
@@ -220,7 +220,7 @@ lldb::SBError SBDebugger::InitializeWithErrorHandling() {
SBError error;
if (auto e = g_debugger_lifetime->Initialize(
std::make_unique<SystemInitializerFull>(), LoadPlugin)) {
- error.SetError(Status(std::move(e)));
+ error.SetError(Status::FromError(std::move(e)));
}
return error;
}
@@ -1360,7 +1360,7 @@ SBError SBDebugger::SetInternalVariable(const char *var_name, const char *value,
"invalid debugger instance name '%s'", debugger_instance_name);
}
if (error.Fail())
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
return sb_error;
}
diff --git a/lldb/source/API/SBError.cpp b/lldb/source/API/SBError.cpp
index 30d9ccc78ee376..31964931649db3 100644
--- a/lldb/source/API/SBError.cpp
+++ b/lldb/source/API/SBError.cpp
@@ -23,7 +23,8 @@ SBError::SBError() { LLDB_INSTRUMENT_VA(this); }
SBError::SBError(const SBError &rhs) {
LLDB_INSTRUMENT_VA(this, rhs);
- m_opaque_up = clone(rhs.m_opaque_up);
+ if (rhs.m_opaque_up)
+ m_opaque_up = std::make_unique<Status>(rhs.m_opaque_up->Clone());
}
SBError::SBError(const char *message) {
@@ -32,8 +33,8 @@ SBError::SBError(const char *message) {
SetErrorString(message);
}
-SBError::SBError(const lldb_private::Status &status)
- : m_opaque_up(new Status(status)) {
+SBError::SBError(lldb_private::Status &&status)
+ : m_opaque_up(new Status(std::move(status))) {
LLDB_INSTRUMENT_VA(this, status);
}
@@ -43,7 +44,9 @@ const SBError &SBError::operator=(const SBError &rhs) {
LLDB_INSTRUMENT_VA(this, rhs);
if (this != &rhs)
- m_opaque_up = clone(rhs.m_opaque_up);
+ if (rhs.m_opaque_up)
+ m_opaque_up = std::make_unique<Status>(rhs.m_opaque_up->Clone());
+
return *this;
}
@@ -111,9 +114,9 @@ void SBError::SetError(uint32_t err, ErrorType type) {
*m_opaque_up = Status(err, type);
}
-void SBError::SetError(const Status &lldb_error) {
+void SBError::SetError(Status &&lldb_error) {
CreateIfNeeded();
- *m_opaque_up = lldb_error;
+ *m_opaque_up = std::move(lldb_error);
}
void SBError::SetErrorToErrno() {
diff --git a/lldb/source/API/SBFile.cpp b/lldb/source/API/SBFile.cpp
index 623708780f4c67..2ae4b1481afbf2 100644
--- a/lldb/source/API/SBFile.cpp
+++ b/lldb/source/API/SBFile.cpp
@@ -62,8 +62,7 @@ SBError SBFile::Read(uint8_t *buf, size_t num_bytes, size_t *bytes_read) {
error = Status::FromErrorString("invalid SBFile");
*bytes_read = 0;
} else {
- Status status = m_opaque_sp->Read(buf, num_bytes);
- error.SetError(status);
+ error.SetError(m_opaque_sp->Read(buf, num_bytes));
*bytes_read = num_bytes;
}
return error;
@@ -78,8 +77,7 @@ SBError SBFile::Write(const uint8_t *buf, size_t num_bytes,
error = Status::FromErrorString("invalid SBFile");
*bytes_written = 0;
} else {
- Status status = m_opaque_sp->Write(buf, num_bytes);
- error.SetError(status);
+ error.SetError(m_opaque_sp->Write(buf, num_bytes));
*bytes_written = num_bytes;
}
return error;
@@ -92,8 +90,7 @@ SBError SBFile::Flush() {
if (!m_opaque_sp) {
error = Status::FromErrorString("invalid SBFile");
} else {
- Status status = m_opaque_sp->Flush();
- error.SetError(status);
+ error.SetError(m_opaque_sp->Flush());
}
return error;
}
@@ -106,10 +103,8 @@ bool SBFile::IsValid() const {
SBError SBFile::Close() {
LLDB_INSTRUMENT_VA(this);
SBError error;
- if (m_opaque_sp) {
- Status status = m_opaque_sp->Close();
- error.SetError(status);
- }
+ if (m_opaque_sp)
+ error.SetError(m_opaque_sp->Close());
return error;
}
diff --git a/lldb/source/API/SBFormat.cpp b/lldb/source/API/SBFormat.cpp
index 51cddceea0372e..080e219d64a362 100644
--- a/lldb/source/API/SBFormat.cpp
+++ b/lldb/source/API/SBFormat.cpp
@@ -36,7 +36,7 @@ SBFormat::SBFormat(const char *format, lldb::SBError &error) {
FormatEntrySP format_entry_sp = std::make_shared<FormatEntity::Entry>();
Status status = FormatEntity::Parse(format, *format_entry_sp);
- error.SetError(status);
+ error.SetError(std::move(status));
if (error.Success())
m_opaque_sp = format_entry_sp;
}
diff --git a/lldb/source/API/SBFrame.cpp b/lldb/source/API/SBFrame.cpp
index b1360e88bcd3af..30c44b974988d0 100644
--- a/lldb/source/API/SBFrame.cpp
+++ b/lldb/source/API/SBFrame.cpp
@@ -809,7 +809,7 @@ SBValueList SBFrame::GetVariables(const lldb::SBVariablesOptions &options) {
Status var_error;
variable_list = frame->GetVariableList(true, &var_error);
if (var_error.Fail())
- value_list.SetError(var_error);
+ value_list.SetError(std::move(var_error));
if (variable_list) {
const size_t num_variables = variable_list->GetSize();
if (num_variables) {
@@ -1033,7 +1033,8 @@ SBValue SBFrame::EvaluateExpression(const char *expr) {
Status error;
error = Status::FromErrorString("can't evaluate expressions when the "
"process is running.");
- ValueObjectSP error_val_sp = ValueObjectConstResult::Create(nullptr, error);
+ ValueObjectSP error_val_sp =
+ ValueObjectConstResult::Create(nullptr, std::move(error));
result.SetSP(error_val_sp, false);
}
return result;
@@ -1129,13 +1130,13 @@ lldb::SBValue SBFrame::EvaluateExpression(const char *expr,
Status error;
error = Status::FromErrorString("can't evaluate expressions when the "
"process is running.");
- expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+ expr_value_sp = ValueObjectConstResult::Create(nullptr, std::move(error));
expr_result.SetSP(expr_value_sp, false);
}
} else {
Status error;
error = Status::FromErrorString("sbframe object is not valid.");
- expr_value_sp = ValueObjectConstResult::Create(nullptr, error);
+ expr_value_sp = ValueObjectConstResult::Create(nullptr, std::move(error));
expr_result.SetSP(expr_value_sp, false);
}
diff --git a/lldb/source/API/SBPlatform.cpp b/lldb/source/API/SBPlatform.cpp
index 2f0f925302f16e..394268b77aa21f 100644
--- a/lldb/source/API/SBPlatform.cpp
+++ b/lldb/source/API/SBPlatform.cpp
@@ -586,7 +586,7 @@ SBProcess SBPlatform::Attach(SBAttachInfo &attach_info,
Status status;
ProcessSP process_sp = platform_sp->Attach(info, debugger.ref(),
target.GetSP().get(), status);
- error.SetError(status);
+ error.SetError(std::move(status));
return SBProcess(process_sp);
}
@@ -728,7 +728,7 @@ SBError SBPlatform::SetLocateModuleCallback(
symbol_file_spec = symbol_file_spec_sb.ref();
}
- return error.ref();
+ return error.ref().Clone();
});
return SBError();
}
diff --git a/lldb/source/API/SBProcess.cpp b/lldb/source/API/SBProcess.cpp
index 3eed51f0245f6f..9773144723c34c 100644
--- a/lldb/source/API/SBProcess.cpp
+++ b/lldb/source/API/SBProcess.cpp
@@ -1450,7 +1450,7 @@ lldb::SBError SBProcess::DeallocateMemory(lldb::addr_t ptr) {
std::lock_guard<std::recursive_mutex> guard(
process_sp->GetTarget().GetAPIMutex());
Status error = process_sp->DeallocateMemory(ptr);
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
} else {
sb_error = Status::FromErrorString("process is running");
}
diff --git a/lldb/source/API/SBSaveCoreOptions.cpp b/lldb/source/API/SBSaveCoreOptions.cpp
index 2cd431611ef558..ef82b0253f1199 100644
--- a/lldb/source/API/SBSaveCoreOptions.cpp
+++ b/lldb/source/API/SBSaveCoreOptions.cpp
@@ -40,8 +40,7 @@ SBSaveCoreOptions::operator=(const SBSaveCoreOptions &rhs) {
SBError SBSaveCoreOptions::SetPluginName(const char *name) {
LLDB_INSTRUMENT_VA(this, name);
- lldb_private::Status error = m_opaque_up->SetPluginName(name);
- return SBError(error);
+ return SBError(m_opaque_up->SetPluginName(name));
}
void SBSaveCoreOptions::SetStyle(lldb::SaveCoreStyle style) {
diff --git a/lldb/source/API/SBStructuredData.cpp b/lldb/source/API/SBStructuredData.cpp
index 801ebf45e0e524..b891a34bd7c76a 100644
--- a/lldb/source/API/SBStructuredData.cpp
+++ b/lldb/source/API/SBStructuredData.cpp
@@ -133,7 +133,7 @@ lldb::SBError SBStructuredData::GetDescription(lldb::SBStream &stream) const {
Status error = m_impl_up->GetDescription(stream.ref());
SBError sb_error;
- sb_error.SetError(error);
+ sb_error.SetError(std::move(error));
return sb_error;
}
diff --git a/lldb/source/API/SBTarget.cpp b/lldb/source/API/SBTarget.cpp
index e927cb854cd88c..1c1f7e2a03def8 100644
--- a/lldb/source/API/SBTarget.cpp
+++ b/lldb/source/API/SBTarget.cpp
@@ -1369,7 +1369,7 @@ SBTarget::WatchpointCreateByAddress(lldb::addr_t addr, size_t size,
CompilerType *type = nullptr;
watchpoint_sp =
target_sp->CreateWatchpoint(addr, size, type, watch_type, cw_error);
- error.SetError(cw_error);
+ error.SetError(std::move(cw_error));
sb_watchpoint.SetSP(watchpo...
[truncated]
|
labath
reviewed
Sep 4, 2024
adrian-prantl
force-pushed
the
status-explicit-clone
branch
from
a269325 to
47e2879
Compare
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
|
I'm not if this is your intention, but the way LLVM has configured their github, your two nicely separated commits will be squashed :( |
|
Ah I've just noticed the other PR |
Do we actually have abetter solution for stacked commits than what I'm doing here? |
short answer is "maybe": https://llvm.org/docs/GitHub.html#using-graphite-for-stacked-pull-requests |
labath
reviewed
Sep 5, 2024
This setup works fine for me, but I think it's worth calling out that you intend it to be viewed that way, as it's not the completely typical way to use github. ( I probably wouldn't have realized it if Felipe did not mention this) |
adrian-prantl
force-pushed
the
status-explicit-clone
branch
3 times, most recently
from
e0a9855 to
8cdcc70
Compare
adrian-prantl
force-pushed
the
status-explicit-clone
branch
from
8cdcc70 to
287735c
Compare
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/5655 Here is the relevant piece of the build log for the reference |
|
LLVM Buildbot has detected a new failure on builder Full details are available at: https://lab.llvm.org/buildbot/#/builders/60/builds/6830 Here is the relevant piece of the build log for the reference |
adrian-prantl
added a commit
to adrian-prantl/llvm-project
that referenced
this pull request
Oct 11, 2024
(cherry picked from commit b798f4b)
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.
Broken out from #106774 for easier review.