[LLDB] Fix error returns in CastToBasicType and CastToEnumType in ValueObject. #117401
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
…ueObject. Update the error returns in ValueObject::CastToBasicType and ValueObject::CastToEnumType to create new errors and return a ValueObjectConstResult with the error, rather tnan updating the error in (and returning) the input ValueObject.
cmtice
requested review from
adrian-prantl
and removed request for
JDevlieghere
|
@llvm/pr-subscribers-lldb Author: None (cmtice) ChangesUpdate the error returns in ValueObject::CastToBasicType and ValueObject::CastToEnumType to create new errors and return a ValueObjectConstResult with the error, rather tnan updating the error in (and returning) the input ValueObject. Full diff: https://github.com/llvm/llvm-project/pull/117401.diff 1 Files Affected:
diff --git a/lldb/source/ValueObject/ValueObject.cpp b/lldb/source/ValueObject/ValueObject.cpp
index 86172ad1b561f9..1093c4b665f4eb 100644
--- a/lldb/source/ValueObject/ValueObject.cpp
+++ b/lldb/source/ValueObject/ValueObject.cpp
@@ -3194,16 +3194,19 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
GetCompilerType().IsPointerType() || GetCompilerType().IsNullPtrType();
bool is_float = GetCompilerType().IsFloat();
bool is_integer = GetCompilerType().IsInteger();
+ ExecutionContext exe_ctx(GetExecutionContextRef());
if (!type.IsScalarType()) {
- m_error = Status::FromErrorString("target type must be a scalar");
- return GetSP();
+ Status error = Status::FromErrorString("target type must be a scalar");
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
if (!is_scalar && !is_enum && !is_pointer) {
- m_error =
+ Status error =
Status::FromErrorString("argument must be a scalar, enum, or pointer");
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
lldb::TargetSP target = GetTargetSP();
@@ -3216,14 +3219,16 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
if (is_pointer) {
if (!type.IsInteger() && !type.IsBoolean()) {
- m_error =
+ Status error =
Status::FromErrorString("target type must be an integer or boolean");
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
if (!type.IsBoolean() && type_byte_size < val_byte_size) {
- m_error = Status::FromErrorString(
+ Status error = Status::FromErrorString(
"target type cannot be smaller than the pointer type");
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
}
@@ -3237,10 +3242,11 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
return ValueObject::CreateValueObjectFromBool(
target, !float_value_or_err->isZero(), "result");
else {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"cannot get value as APFloat: %s",
llvm::toString(float_value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
}
}
@@ -3256,11 +3262,12 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
return ValueObject::CreateValueObjectFromAPInt(target, ext, type,
"result");
} else {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"cannot get value as APSInt: %s",
llvm::toString(int_value_or_err.takeError()).c_str());
;
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
} else if (is_scalar && is_float) {
llvm::APSInt integer(type_byte_size * CHAR_BIT, !type.IsSigned());
@@ -3274,10 +3281,11 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
// Casting floating point values that are out of bounds of the target
// type is undefined behaviour.
if (status & llvm::APFloatBase::opInvalidOp) {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"invalid type cast detected: %s",
llvm::toString(float_value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
return ValueObject::CreateValueObjectFromAPInt(target, integer, type,
"result");
@@ -3297,10 +3305,11 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
return ValueObject::CreateValueObjectFromAPFloat(target, f, type,
"result");
} else {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"cannot get value as APSInt: %s",
llvm::toString(int_value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
} else {
if (is_integer) {
@@ -3312,10 +3321,11 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
return ValueObject::CreateValueObjectFromAPFloat(target, f, type,
"result");
} else {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"cannot get value as APSInt: %s",
llvm::toString(int_value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
}
if (is_float) {
@@ -3327,33 +3337,38 @@ lldb::ValueObjectSP ValueObject::CastToBasicType(CompilerType type) {
return ValueObject::CreateValueObjectFromAPFloat(target, f, type,
"result");
} else {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"cannot get value as APFloat: %s",
llvm::toString(float_value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
}
}
}
- m_error = Status::FromErrorString("Unable to perform requested cast");
- return GetSP();
+ Status error = Status::FromErrorString("Unable to perform requested cast");
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
lldb::ValueObjectSP ValueObject::CastToEnumType(CompilerType type) {
bool is_enum = GetCompilerType().IsEnumerationType();
bool is_integer = GetCompilerType().IsInteger();
bool is_float = GetCompilerType().IsFloat();
+ ExecutionContext exe_ctx(GetExecutionContextRef());
if (!is_enum && !is_integer && !is_float) {
- m_error = Status::FromErrorString(
+ Status error = Status::FromErrorString(
"argument must be an integer, a float, or an enum");
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
if (!type.IsEnumerationType()) {
- m_error = Status::FromErrorString("target type must be an enum");
- return GetSP();
+ Status error = Status::FromErrorString("target type must be an enum");
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
lldb::TargetSP target = GetTargetSP();
@@ -3372,16 +3387,18 @@ lldb::ValueObjectSP ValueObject::CastToEnumType(CompilerType type) {
// Casting floating point values that are out of bounds of the target
// type is undefined behaviour.
if (status & llvm::APFloatBase::opInvalidOp) {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"invalid type cast detected: %s",
llvm::toString(value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
return ValueObject::CreateValueObjectFromAPInt(target, integer, type,
"result");
} else {
- m_error = Status::FromErrorString("cannot get value as APFloat");
- return GetSP();
+ Status error = Status::FromErrorString("cannot get value as APFloat");
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
} else {
// Get the value as APSInt and extend or truncate it to the requested size.
@@ -3391,14 +3408,16 @@ lldb::ValueObjectSP ValueObject::CastToEnumType(CompilerType type) {
return ValueObject::CreateValueObjectFromAPInt(target, ext, type,
"result");
} else {
- m_error = Status::FromErrorStringWithFormat(
+ Status error = Status::FromErrorStringWithFormat(
"cannot get value as APSInt: %s",
llvm::toString(value_or_err.takeError()).c_str());
- return GetSP();
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
}
- m_error = Status::FromErrorString("Cannot perform requested cast");
- return GetSP();
+ Status error = Status::FromErrorString("Cannot perform requested cast");
+ return ValueObjectConstResult::Create(
+ exe_ctx.GetBestExecutionContextScope(), error.Clone());
}
ValueObject::EvaluationPoint::EvaluationPoint() : m_mod_id(), m_exe_ctx_ref() {}
|
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
labath
reviewed
Dec 2, 2024
Comment on lines
3200
to
3202
| Status error = Status::FromErrorString("target type must be a scalar"); | ||
| return ValueObjectConstResult::Create( | ||
| exe_ctx.GetBestExecutionContextScope(), error.Clone()); |
There was a problem hiding this comment.
Suggested change
| Status error = Status::FromErrorString("target type must be a scalar"); | |
| return ValueObjectConstResult::Create( | |
| exe_ctx.GetBestExecutionContextScope(), error.Clone()); | |
| return ValueObjectConstResult::Create( | |
| exe_ctx.GetBestExecutionContextScope(), Status::FromErrorString("target type must be a scalar")); |
If you really want to have a variable for this, then you can keep what you have and use std::move instead of Clone, but I think this is better (and I think Adrian would agree :) ).
This comment applies throughout the patch.
labath
approved these changes
Dec 4, 2024
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.
Update the error returns in ValueObject::CastToBasicType and ValueObject::CastToEnumType to create new errors and return a ValueObjectConstResult with the error, rather tnan updating the error in (and returning) the input ValueObject.