[Lldb-commits] [PATCH] D71498: Fix ARM32 inferior calls

[Greg Clayton via Phabricator via lldb-commits]

clayborg added a comment.

> In other places you're replacing a reinterpret_cast<addr_t> with two c casts. I guess this was done because two c++ casts were too long (?). In these cases the second cast is not really needed, as uintptr_t->addr_t should convert automatically. I think I'd prefer that we just have a single reinterpret_cast<uintptr_t> instead of two c casts. Then our rule can be "always convert a pointer to uintptr_t". I don't know if there's a clang-tidy check for that, but it sounds like that could be something which could be checked/enforced there...

For the printf style statement, we can't use just one cast to "uintptr_t" because on 32 bit systems it won't be converted to 64 bit. If we switch for LLDB_LOG() that uses the llvm formats, we are good, but not with printf



================
Comment at: lldb/source/Expression/IRExecutionUnit.cpp:351-352
+    m_jitted_functions.push_back(
+        JittedFunction(function.getName().str().c_str(), external,
+                       (lldb::addr_t)(uintptr_t)fun_ptr));
   }
----------------
This can probably just be:

```
 JittedFunction(function.getName().str().c_str(), external, (uintptr_t)fun_ptr));
```
since this is a function call and "fun_ptr" will be correctly converted to a lldb::addr_t


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71498