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



_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits

Reply via email to