teemperor accepted this revision.
teemperor added a comment.
This revision is now accepted and ready to land.
I think this is the last round of review so I'll just accept this modulo a few
nits.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:190
+
+ StructuredData::ObjectSP structured_object(
+ py_return.CreateStructuredObject());
----------------
As discussed offline, I think with the API we got here I would much rather have
the unhandled llvm::Expected from the old code than the strange StructuredData
code that just silently hides errors. Let's revert this back to the old but
with llvm::None and put a big FIXME there.
```
lang=c++
llvm::Expected<unsigned long long> size = py_return.AsUnsignedLongLong();
if (!size) {
// FIXME: Handle error.
return llvm::None;
}
return *size;
```
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:299
lldb::pid_t ScriptedProcessPythonInterface::GetProcessID() {
- size_t pid = GetGenericInteger("get_process_id");
-
- return (pid >= std::numeric_limits<lldb::pid_t>::max())
- ? LLDB_INVALID_PROCESS_ID
- : pid;
+ auto pid = GetGenericInteger("get_process_id");
+ return (!pid) ? LLDB_INVALID_PROCESS_ID : *pid;
----------------
Could you spell the `auto` out here? `llvm::Optional<uint64_t>` is I think
simple enough.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:304
bool ScriptedProcessPythonInterface::IsAlive() {
- return GetGenericInteger("is_alive");
+ auto is_alive = GetGenericInteger("is_alive");
+
----------------
Same as above.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D105788/new/
https://reviews.llvm.org/D105788
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
