Magic-Mirror/llvm-capstone
1
0
Fork 0
mirror of https://github.com/capstone-engine/llvm-capstone.git synced 2024-11-26 23:21:11 +00:00
Code Issues Actions 2 Packages Projects Releases Wiki Activity

[lldb] Improve maintainability and readability for ValueObject methods (#75865)

Browse Source 
As I worked through changes to another PR
(https://github.com/llvm/llvm-project/pull/74912), I couldn't help but
rewrite a few methods for readability, maintainability, and possibly
some behavior correctness too.

1. Exiting early instead of nested `if`-statements, which:
	- Reduces indentation levels for all subsequent lines
	- Treats missing pre-conditions similar to an error
- Clearly indicates that the full length of the method is the "happy
path".
2. Explicitly return empty Value Object shared pointers for those error
(like) situations, which
- Reduces the time it takes a maintainer to figure out what the method
actually returns based on those conditions.

3. Converting a mix of `if` and `if`-`else`-statements around an enum
into one `switch` statement, which:
	- Consolidates the former branching logic
	- Lets the compiler warn you of a (future) missing enum case
- This one may actually change behavior slightly, because what was an
early test for one enum case, now happens later on in the `switch`.

4. Consolidating near-identical, "copy-pasta" logic into one place,
which:
	- Separates the common code to the diverging paths.
	- Highlights the differences between the code paths.



rdar://119833526
This commit is contained in:
Pete Lawrence 2024-01-23 14:07:52 -10:00 committed by GitHub
parent 25e1916d88
commit d657519838
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
1 changed files with 158 additions and 160 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

318
lldb/source/Core/ValueObject.cpp
View File

@ -1582,62 +1582,64 @@ bool ValueObject::IsUninitializedReference() {
ValueObjectSP ValueObject::GetSyntheticArrayMember(size_t index,
bool can_create) {
ValueObjectSP synthetic_child_sp;
if (IsPointerType() || IsArrayType()) {
std::string index_str = llvm::formatv("[{0}]", index);
ConstString index_const_str(index_str);
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
synthetic_child_sp = GetSyntheticChild(index_const_str);
if (!synthetic_child_sp) {
ValueObject *synthetic_child;
// We haven't made a synthetic array member for INDEX yet, so lets make
// one and cache it for any future reference.
synthetic_child = CreateChildAtIndex(0, true, index);
if (!IsPointerType() && !IsArrayType())
return ValueObjectSP();
// Cache the value if we got one back...
if (synthetic_child) {
AddSyntheticChild(index_const_str, synthetic_child);
synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(ConstString(index_str));
synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true;
}
}
}
std::string index_str = llvm::formatv("[{0}]", index);
ConstString index_const_str(index_str);
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
if (auto existing_synthetic_child = GetSyntheticChild(index_const_str))
return existing_synthetic_child;
// We haven't made a synthetic array member for INDEX yet, so lets make
// one and cache it for any future reference.
ValueObject *synthetic_child = CreateChildAtIndex(0, true, index);
if (!synthetic_child)
return ValueObjectSP();
// Cache the synthetic child's value because it's valid.
AddSyntheticChild(index_const_str, synthetic_child);
auto synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(ConstString(index_str));
synthetic_child_sp->m_flags.m_is_array_item_for_pointer = true;
return synthetic_child_sp;
}
ValueObjectSP ValueObject::GetSyntheticBitFieldChild(uint32_t from, uint32_t to,
bool can_create) {
ValueObjectSP synthetic_child_sp;
if (IsScalarType()) {
std::string index_str = llvm::formatv("[{0}-{1}]", from, to);
ConstString index_const_str(index_str);
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
synthetic_child_sp = GetSyntheticChild(index_const_str);
if (!synthetic_child_sp) {
uint32_t bit_field_size = to - from + 1;
uint32_t bit_field_offset = from;
if (GetDataExtractor().GetByteOrder() == eByteOrderBig)
bit_field_offset =
GetByteSize().value_or(0) * 8 - bit_field_size - bit_field_offset;
// We haven't made a synthetic array member for INDEX yet, so lets make
// one and cache it for any future reference.
ValueObjectChild *synthetic_child = new ValueObjectChild(
*this, GetCompilerType(), index_const_str, GetByteSize().value_or(0),
0, bit_field_size, bit_field_offset, false, false,
eAddressTypeInvalid, 0);
if (!IsScalarType())
return ValueObjectSP();
// Cache the value if we got one back...
if (synthetic_child) {
AddSyntheticChild(index_const_str, synthetic_child);
synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(ConstString(index_str));
synthetic_child_sp->m_flags.m_is_bitfield_for_scalar = true;
}
}
}
std::string index_str = llvm::formatv("[{0}-{1}]", from, to);
ConstString index_const_str(index_str);
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
if (auto existing_synthetic_child = GetSyntheticChild(index_const_str))
return existing_synthetic_child;
uint32_t bit_field_size = to - from + 1;
uint32_t bit_field_offset = from;
if (GetDataExtractor().GetByteOrder() == eByteOrderBig)
bit_field_offset =
GetByteSize().value_or(0) * 8 - bit_field_size - bit_field_offset;
// We haven't made a synthetic array member for INDEX yet, so lets make
// one and cache it for any future reference.
ValueObjectChild *synthetic_child = new ValueObjectChild(
*this, GetCompilerType(), index_const_str, GetByteSize().value_or(0), 0,
bit_field_size, bit_field_offset, false, false, eAddressTypeInvalid, 0);
if (!synthetic_child)
return ValueObjectSP();
// Cache the synthetic child's value because it's valid.
AddSyntheticChild(index_const_str, synthetic_child);
auto synthetic_child_sp = synthetic_child->GetSP();
synthetic_child_sp->SetName(ConstString(index_str));
synthetic_child_sp->m_flags.m_is_bitfield_for_scalar = true;
return synthetic_child_sp;
}
@ -1647,9 +1649,8 @@ ValueObjectSP ValueObject::GetSyntheticChildAtOffset(
ValueObjectSP synthetic_child_sp;
if (name_const_str.IsEmpty()) {
if (name_const_str.IsEmpty())
name_const_str.SetString("@" + std::to_string(offset));
}
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
@ -1659,13 +1660,13 @@ ValueObjectSP ValueObject::GetSyntheticChildAtOffset(
return synthetic_child_sp;
if (!can_create)
return {};
return ValueObjectSP();
ExecutionContext exe_ctx(GetExecutionContextRef());
std::optional<uint64_t> size =
type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
if (!size)
return {};
return ValueObjectSP();
ValueObjectChild *synthetic_child =
new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
false, false, eAddressTypeInvalid, 0);
@ -1699,7 +1700,7 @@ ValueObjectSP ValueObject::GetSyntheticBase(uint32_t offset,
return synthetic_child_sp;
if (!can_create)
return {};
return ValueObjectSP();
const bool is_base_class = true;
@ -1707,7 +1708,7 @@ ValueObjectSP ValueObject::GetSyntheticBase(uint32_t offset,
std::optional<uint64_t> size =
type.GetByteSize(exe_ctx.GetBestExecutionContextScope());
if (!size)
return {};
return ValueObjectSP();
ValueObjectChild *synthetic_child =
new ValueObjectChild(*this, type, name_const_str, *size, offset, 0, 0,
is_base_class, false, eAddressTypeInvalid, 0);
@ -1736,30 +1737,30 @@ static const char *SkipLeadingExpressionPathSeparators(const char *expression) {
ValueObjectSP
ValueObject::GetSyntheticExpressionPathChild(const char *expression,
bool can_create) {
ValueObjectSP synthetic_child_sp;
ConstString name_const_string(expression);
// Check if we have already created a synthetic array member in this valid
// object. If we have we will re-use it.
synthetic_child_sp = GetSyntheticChild(name_const_string);
if (!synthetic_child_sp) {
// We haven't made a synthetic array member for expression yet, so lets
// make one and cache it for any future reference.
synthetic_child_sp = GetValueForExpressionPath(
expression, nullptr, nullptr,
GetValueForExpressionPathOptions().SetSyntheticChildrenTraversal(
GetValueForExpressionPathOptions::SyntheticChildrenTraversal::
None));
if (auto existing_synthetic_child = GetSyntheticChild(name_const_string))
return existing_synthetic_child;
// Cache the value if we got one back...
if (synthetic_child_sp.get()) {
// FIXME: this causes a "real" child to end up with its name changed to
// the contents of expression
AddSyntheticChild(name_const_string, synthetic_child_sp.get());
synthetic_child_sp->SetName(
ConstString(SkipLeadingExpressionPathSeparators(expression)));
}
}
return synthetic_child_sp;
// We haven't made a synthetic array member for expression yet, so lets
// make one and cache it for any future reference.
auto path_options = GetValueForExpressionPathOptions();
path_options.SetSyntheticChildrenTraversal(
GetValueForExpressionPathOptions::SyntheticChildrenTraversal::None);
auto synthetic_child =
GetValueForExpressionPath(expression, nullptr, nullptr, path_options);
if (!synthetic_child)
return ValueObjectSP();
// Cache the synthetic child's value because it's valid.
// FIXME: this causes a "real" child to end up with its name changed to
// the contents of expression
AddSyntheticChild(name_const_string, synthetic_child.get());
synthetic_child->SetName(
ConstString(SkipLeadingExpressionPathSeparators(expression)));
return synthetic_child;
}
void ValueObject::CalculateSyntheticValue() {
@ -1956,66 +1957,55 @@ ValueObjectSP ValueObject::GetValueForExpressionPath(
const GetValueForExpressionPathOptions &options,
ExpressionPathAftermath *final_task_on_target) {
ExpressionPathScanEndReason dummy_reason_to_stop =
ValueObject::eExpressionPathScanEndReasonUnknown;
ExpressionPathEndResultType dummy_final_value_type =
ValueObject::eExpressionPathEndResultTypeInvalid;
ExpressionPathAftermath dummy_final_task_on_target =
ValueObject::eExpressionPathAftermathNothing;
auto dummy_stop_reason = eExpressionPathScanEndReasonUnknown;
auto dummy_value_type = eExpressionPathEndResultTypeInvalid;
auto dummy_final_task = eExpressionPathAftermathNothing;
ValueObjectSP ret_val = GetValueForExpressionPath_Impl(
expression, reason_to_stop ? reason_to_stop : &dummy_reason_to_stop,
final_value_type ? final_value_type : &dummy_final_value_type, options,
final_task_on_target ? final_task_on_target
: &dummy_final_task_on_target);
auto proxy_stop_reason = reason_to_stop ? reason_to_stop : &dummy_stop_reason;
auto proxy_value_type =
final_value_type ? final_value_type : &dummy_value_type;
auto proxy_final_task =
final_task_on_target ? final_task_on_target : &dummy_final_task;
if (!final_task_on_target ||
*final_task_on_target == ValueObject::eExpressionPathAftermathNothing)
return ret_val;
auto ret_value = GetValueForExpressionPath_Impl(expression, proxy_stop_reason,
proxy_value_type, options,
proxy_final_task);
if (ret_val.get() &&
((final_value_type ? *final_value_type : dummy_final_value_type) ==
eExpressionPathEndResultTypePlain)) // I can only deref and takeaddress
// of plain objects
{
if ((final_task_on_target ? *final_task_on_target
: dummy_final_task_on_target) ==
ValueObject::eExpressionPathAftermathDereference) {
Status error;
ValueObjectSP final_value = ret_val->Dereference(error);
if (error.Fail() || !final_value.get()) {
if (reason_to_stop)
*reason_to_stop =
ValueObject::eExpressionPathScanEndReasonDereferencingFailed;
if (final_value_type)
*final_value_type = ValueObject::eExpressionPathEndResultTypeInvalid;
return ValueObjectSP();
} else {
if (final_task_on_target)
*final_task_on_target = ValueObject::eExpressionPathAftermathNothing;
return final_value;
}
}
if (*final_task_on_target ==
ValueObject::eExpressionPathAftermathTakeAddress) {
Status error;
ValueObjectSP final_value = ret_val->AddressOf(error);
if (error.Fail() || !final_value.get()) {
if (reason_to_stop)
*reason_to_stop =
ValueObject::eExpressionPathScanEndReasonTakingAddressFailed;
if (final_value_type)
*final_value_type = ValueObject::eExpressionPathEndResultTypeInvalid;
return ValueObjectSP();
} else {
if (final_task_on_target)
*final_task_on_target = ValueObject::eExpressionPathAftermathNothing;
return final_value;
}
}
// The caller knows nothing happened if `final_task_on_target` doesn't change.
if (!ret_value || (*proxy_value_type) != eExpressionPathEndResultTypePlain ||
!final_task_on_target)
return ValueObjectSP();
ExpressionPathAftermath &final_task_on_target_ref = (*final_task_on_target);
ExpressionPathScanEndReason stop_reason_for_error;
Status error;
// The method can only dereference and take the address of plain objects.
switch (final_task_on_target_ref) {
case eExpressionPathAftermathNothing:
return ret_value;
case eExpressionPathAftermathDereference:
ret_value = ret_value->Dereference(error);
stop_reason_for_error = eExpressionPathScanEndReasonDereferencingFailed;
break;
case eExpressionPathAftermathTakeAddress:
ret_value = ret_value->AddressOf(error);
stop_reason_for_error = eExpressionPathScanEndReasonTakingAddressFailed;
break;
}
return ret_val; // final_task_on_target will still have its original value, so
// you know I did not do it
if (ret_value && error.Success()) {
final_task_on_target_ref = eExpressionPathAftermathNothing;
return ret_value;
}
if (reason_to_stop)
*reason_to_stop = stop_reason_for_error;
if (final_value_type)
*final_value_type = eExpressionPathEndResultTypeInvalid;
return ValueObjectSP();
}
ValueObjectSP ValueObject::GetValueForExpressionPath_Impl(
@ -2686,39 +2676,47 @@ ValueObjectSP ValueObject::AddressOf(Status &error) {
const bool scalar_is_load_address = false;
addr_t addr = GetAddressOf(scalar_is_load_address, &address_type);
error.Clear();
if (addr != LLDB_INVALID_ADDRESS && address_type != eAddressTypeHost) {
switch (address_type) {
case eAddressTypeInvalid: {
StreamString expr_path_strm;
GetExpressionPath(expr_path_strm);
error.SetErrorStringWithFormat("'%s' is not in memory",
expr_path_strm.GetData());
} break;
case eAddressTypeFile:
case eAddressTypeLoad: {
CompilerType compiler_type = GetCompilerType();
if (compiler_type) {
std::string name(1, '&');
name.append(m_name.AsCString(""));
ExecutionContext exe_ctx(GetExecutionContextRef());
m_addr_of_valobj_sp = ValueObjectConstResult::Create(
exe_ctx.GetBestExecutionContextScope(),
compiler_type.GetPointerType(), ConstString(name.c_str()), addr,
eAddressTypeInvalid, m_data.GetAddressByteSize());
}
} break;
default:
break;
}
} else {
StreamString expr_path_strm;
GetExpressionPath(expr_path_strm);
StreamString expr_path_strm;
GetExpressionPath(expr_path_strm);
const char *expr_path_str = expr_path_strm.GetData();
ExecutionContext exe_ctx(GetExecutionContextRef());
auto scope = exe_ctx.GetBestExecutionContextScope();
if (addr == LLDB_INVALID_ADDRESS) {
error.SetErrorStringWithFormat("'%s' doesn't have a valid address",
expr_path_strm.GetData());
expr_path_str);
return ValueObjectSP();
}
return m_addr_of_valobj_sp;
switch (address_type) {
case eAddressTypeInvalid:
error.SetErrorStringWithFormat("'%s' is not in memory", expr_path_str);
return ValueObjectSP();
case eAddressTypeHost:
error.SetErrorStringWithFormat("'%s' is in host process (LLDB) memory",
expr_path_str);
return ValueObjectSP();
case eAddressTypeFile:
case eAddressTypeLoad: {
CompilerType compiler_type = GetCompilerType();
if (!compiler_type) {
error.SetErrorStringWithFormat("'%s' doesn't have a compiler type",
expr_path_str);
return ValueObjectSP();
}
std::string name(1, '&');
name.append(m_name.AsCString(""));
m_addr_of_valobj_sp = ValueObjectConstResult::Create(
scope, compiler_type.GetPointerType(), ConstString(name.c_str()), addr,
eAddressTypeInvalid, m_data.GetAddressByteSize());
return m_addr_of_valobj_sp;
}
}
}
ValueObjectSP ValueObject::DoCast(const CompilerType &compiler_type) {