mib marked 4 inline comments as done.
mib added inline comments.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:66
bool ScriptedProcessPythonInterface::ShouldStop() {
- return GetGenericInteger("shuold_stop");
+ return static_cast<bool>(GetGenericInteger("shuold_stop"));
}
----------------
teemperor wrote:
> Unrelated to the change here, but that looks like a typo in the string.
Thanks for catching that!
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.cpp:294
bool ScriptedProcessPythonInterface::IsAlive() {
- return GetGenericInteger("is_alive");
+ return static_cast<bool>(GetGenericInteger("is_alive"));
}
----------------
teemperor wrote:
> I think this should return `false` when the underlying object is invalid? Bit
> out of scope for this patch, so feel free to fix this in another review.
This is just the interface between Python and C++, the object sanity check is
done above in `ScriptedProcess::IsAlive`.
================
Comment at:
lldb/source/Plugins/ScriptInterpreter/Python/ScriptedProcessPythonInterface.h:54
protected:
- size_t GetGenericInteger(llvm::StringRef method_name);
+ unsigned long long GetGenericInteger(llvm::StringRef method_name);
Status GetStatusFromMethod(llvm::StringRef method_name);
----------------
teemperor wrote:
> Could you actually make a typedef for this? `typedef unsigned long long
> PythonInteger` or so. cpython really like to change their API between
> versions and this would make the potential fixups a bit easier if they decide
> to change the return value.
>
> We might actually want to put this typedef into the `PythonObject` API int
> the future, but that's probably out-of-scope for this patch.
There is actually a `PythonInteger` class already but in order to extract the
scalar, I have to make a proxy `StructuredData::Integer` to get the `uint64_t`
value. Notice, in this case, the return type is not `unsigned long long`
anymore ...
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
