[Lldb-commits] [PATCH] D84267: Thread ExecutionContextScope through GetByteSize where possible (NFC-ish)

[Jim Ingham via Phabricator via lldb-commits]

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