bulbazord added a comment.
This change seems mostly red in that we're removing a lot of things. What are
the replacements? I also see you're removing the API lock acquisition in a few
places, where does that now occur?
================
Comment at: lldb/bindings/python/python-wrapper.swig:266-279
+ switch (arg_info.get().max_positional_args) {
+ case 1:
+ // FIXME: Since this is used by different scripting affordances, they
can have different number
+ // of argument but also different types of arguments (i.e
SBExecutionContect vs SBProcess)
+ // We need to have a more reliable way to forward positional arguments.
+ result = pfunc(SWIGBridge::ToSWIGWrapper(exe_ctx_sp->GetProcessSP()));
+ break;
----------------
Is there no way to check the types of the positional args? This seems like it
might be a source of future subtle bugs or unexpected behavior.
================
Comment at:
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp:112-114
+ // return llvm::createStringError(
+ // llvm::inconvertibleErrorCode(),
+ // "Failed to create scripted thread interface.");
----------------
nit: Remove commented out code.
side note, maybe OperatingSystemPython should have a static factory method so
we can actually return a meaningful error like this? Since the constructor can
fail, we might end up with a half-initialized object. Doesn't have to be in
this change, I think it would make sense for a follow-up.
================
Comment at:
lldb/source/Plugins/OperatingSystem/Python/OperatingSystemPython.cpp:123-128
+ // return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ // "Failed to create script object.");
+ return;
+ if (!owned_script_object_sp->IsValid())
+ // return llvm::createStringError(llvm::inconvertibleErrorCode(),
+ // "Created script object is invalid.");
----------------
Same here
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D159314/new/
https://reviews.llvm.org/D159314
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
