shafik added a comment.
Sorry for the late review
================
Comment at: lldb/trunk/include/lldb/Target/ProcessStructReader.h:70
}
- size_t total_size = struct_type.GetByteSize(nullptr);
- lldb::DataBufferSP buffer_sp(new DataBufferHeap(total_size, 0));
+ auto total_size = struct_type.GetByteSize(nullptr);
+ if (!total_size)
----------------
I will also second the sentiment on the `auto`, it does not improve readability.
================
Comment at: lldb/trunk/source/API/SBType.cpp:106
uint64_t SBType::GetByteSize() {
- if (!IsValid())
- return 0;
-
- return m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr);
+ if (IsValid())
+ if (auto size = m_opaque_sp->GetCompilerType(false).GetByteSize(nullptr))
----------------
The early exit is more readable here and also has a lower mental load.
================
Comment at: lldb/trunk/source/Commands/CommandObjectMemory.cpp:647
+ if (!size) {
+ result.AppendError("can't get size of type");
+ return false;
----------------
This does not appear to be NFC
================
Comment at: lldb/trunk/source/Commands/CommandObjectMemory.cpp:1069
+ if (!size)
+ return false;
+ switch (*size) {
----------------
NFC?
================
Comment at: lldb/trunk/source/Core/Value.cpp:227
if (ast_type.IsValid())
- byte_size = ast_type.GetByteSize(
- exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr);
+ if (auto size = ast_type.GetByteSize(
+ exe_ctx ? exe_ctx->GetBestExecutionContextScope() : nullptr))
----------------
This might read better as two steps, get the value and then check it.
================
Comment at: lldb/trunk/source/Expression/Materializer.cpp:50
+ if (auto size = type.GetByteSize(nullptr))
+ m_size = *size;
----------------
What value does `m_size` have otherwise?
================
Comment at: lldb/trunk/source/Expression/Materializer.cpp:800
+ if (!byte_size) {
+ err.SetErrorString("can't get size of type");
+ return;
----------------
NFC?
================
Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-arm64/ABIMacOSX_arm64.cpp:2372
lldb::offset_t offset = 0;
- if (byte_size == sizeof(float)) {
+ if (*byte_size == sizeof(float)) {
value.GetScalar() = data.GetFloat(&offset);
----------------
switch?
================
Comment at: lldb/trunk/source/Plugins/ABI/MacOSX-i386/ABIMacOSX_i386.cpp:827
CompilerType compiler_type(value->GetCompilerType());
- if (compiler_type) {
+ auto bit_size = compiler_type.GetBitSize(&thread);
+ if (bit_size) {
----------------
Looks like you chopped out the `if (compiler_type) ` check
================
Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:1720
if (homogeneous_count > 0 && homogeneous_count <= 4) {
+ auto base_byte_size = base_type.GetByteSize(nullptr);
if (base_type.IsVectorType(nullptr, nullptr)) {
----------------
There are a lot of checks for `base_byte_size` so it is not clear what value
`vfp_byte_size` will have if `base_byte_size` is empty.
The flow looks a lot different but hard to track what will be set and not set.
================
Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm/ABISysV_arm.cpp:1788
- DataBufferSP data_sp(new DataBufferHeap(byte_size, 0));
+ DataBufferSP data_sp(new DataBufferHeap(*byte_size, 0));
uint32_t data_offset = 0;
----------------
`make_shared`?
================
Comment at: lldb/trunk/source/Plugins/ABI/SysV-arm64/ABISysV_arm64.cpp:2343
lldb::offset_t offset = 0;
- if (byte_size == sizeof(float)) {
+ if (*byte_size == sizeof(float)) {
value.GetScalar() = data.GetFloat(&offset);
----------------
switch?
================
Comment at: lldb/trunk/source/Plugins/ABI/SysV-s390x/ABISysV_s390x.cpp:379
CompilerType compiler_type = value->GetCompilerType();
- if (!compiler_type)
+ auto bit_size = compiler_type.GetBitSize(&thread);
+ if (!bit_size)
----------------
Looks the `if (!compiler_type)` was removed.
================
Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:228
}
- CompilerType
pair_type(__i_->GetCompilerType().GetTypeTemplateArgument(0));
- std::string name; uint64_t bit_offset_ptr; uint32_t
bitfield_bit_size_ptr; bool is_bitfield_ptr;
- pair_type = pair_type.GetFieldAtIndex(0, name, &bit_offset_ptr,
&bitfield_bit_size_ptr, &is_bitfield_ptr);
+ CompilerType pair_type(
+ __i_->GetCompilerType().GetTypeTemplateArgument(0));
----------------
Unrelated formatting changes?
================
Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:243
m_pair_ptr = nullptr;
- if (addr && addr!=LLDB_INVALID_ADDRESS) {
- ClangASTContext *ast_ctx =
llvm::dyn_cast_or_null<ClangASTContext>(pair_type.GetTypeSystem());
+ if (addr && addr != LLDB_INVALID_ADDRESS) {
+ ClangASTContext *ast_ctx =
----------------
Unrelated formatting changes?
================
Comment at: lldb/trunk/source/Plugins/Language/CPlusPlus/LibCxx.cpp:268
return false;
- DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
process_sp->GetAddressByteSize());
- auto pair_sp = CreateValueObjectFromData("pair", extractor,
valobj_sp->GetExecutionContextRef(), tree_node_type);
+ DataExtractor extractor(buffer_sp, process_sp->GetByteOrder(),
+ process_sp->GetAddressByteSize());
----------------
Formatting?
Repository:
rL LLVM
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56688/new/
https://reviews.llvm.org/D56688
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
