jingham added a comment.
I pointed out a couple of places where you have either at hand or near at hand
a useful exe_ctx that you could pass instead of nullptr.
Other than that, this seems good to me. Thanks for pushing this through.
================
Comment at: lldb/source/API/SBType.cpp:210
SBType SBType::GetArrayElementType() {
LLDB_RECORD_METHOD_NO_ARGS(lldb::SBType, SBType, GetArrayElementType);
----------------
This changes makes it obvious we should add
SBType::GetByteSize(SBExecutionContext &exe_ctx)
SBType::GetArrayElementType(SBExecutionContext &exe_ctx)
but I don't think you need to do that in this patch.
================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1655
type_sp->GetFullCompilerType();
- type_sp->GetDescription(&strm, eDescriptionLevelFull, true);
+ type_sp->GetDescription(&strm, eDescriptionLevelFull, true, nullptr);
// Print all typedef chains
----------------
LookupTypeInModule is called by LookupInModule which is only ever called when
we have a target on hand. Maybe it would be better to thread the target
through so we can get better type descriptions if the target has a live
process? Or make it a member function of CommandObjectTargetModulesLookup and
it could use m_exe_ctx. It doesn't seem to be gaining anything by being a
stand-alone function.
================
Comment at: lldb/source/Commands/CommandObjectTarget.cpp:1700
type_sp->GetFullCompilerType();
- type_sp->GetDescription(&strm, eDescriptionLevelFull, true);
- // Print all typedef chains
+ type_sp->GetDescription(&strm, eDescriptionLevelFull, true, nullptr);
+ // Print all typedef chains.
----------------
Ditto with this one. Our only caller (the method called LookupTypeHere)
definitely had a better ExecutionContextScope (from m_exe_ctx). Be nice to
pass it in here.
================
Comment at: lldb/source/DataFormatters/FormatManager.cpp:240
if (compiler_type.IsArrayType(nullptr, &array_size, nullptr)) {
- CompilerType element_type = compiler_type.GetArrayElementType();
+ CompilerType element_type = compiler_type.GetArrayElementType(nullptr);
if (element_type.IsTypedefType()) {
----------------
We have the ValueObject we are working on in this scope. ValueObjects always
have an ExecutionContextRef (GetExecutionContextRef) so you can pass in a
better ExecutionContextScope here.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D84267/new/
https://reviews.llvm.org/D84267
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
