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

[Adrian Prantl via Phabricator via lldb-commits]

aprantl marked 3 inline comments as done.
aprantl added inline comments.


================
Comment at: lldb/source/Plugins/ExpressionParser/Clang/IRForTarget.cpp:308
   lldb::TargetSP target_sp(m_execution_unit.GetTarget());
-  lldb_private::ExecutionContext exe_ctx(target_sp, true);
-  llvm::Optional<uint64_t> bit_size =
-      m_result_type.GetBitSize(exe_ctx.GetBestExecutionContextScope());
+  llvm::Optional<uint64_t> bit_size = m_result_type.GetBitSize(target_sp.get());
   if (!bit_size) {
----------------
shafik wrote:
> Is this equivalent to what was being done previously or is this better in some way?
The old code took a `TargetSP`, wrapped it in an `ExecutionContext`, and then called `GetBestExecutionContextScope()`, which returns the `Target *`. (`Target` inherits from `ExecutionContextScope`)

So the new code does exactly the same thing as the old code but it's much shorter.


================
Comment at: lldb/source/Plugins/TypeSystem/Clang/TypeSystemClang.cpp:4092
 TypeSystemClang::GetArrayElementType(lldb::opaque_compiler_type_t type,
-                                     uint64_t *stride) {
   if (type) {
----------------
teemperor wrote:
> So we already have this dubious stride extra-output here, but now it also requires us to have an execution context parameter. I think we might as well just remove that parameter (in a separate patch).
Are you saying no one uses `stride`?

Not even a future Fortran lldb Plugin?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D84267/new/

https://reviews.llvm.org/D84267