clayborg added a comment.
Looks good overall. A few questions:
- should we do a global "s/extra_args/args/" edit in this patch? Not sure what
"extra_" is adding? Can we remove?
- Can the structured data not be a dictionary? Valid StructuredData could be
"malloc" or "[0x1000, 0x2000]" or "true", etc. If the data is required to be a
dictionary we should provide lldb::SBError support for the new API calls to
ensure we can let the user know what they did wrong. Be good to test this as
well.
- is it possible to update the args for a callback function? That would seem
like a good thing you might want to do? And if so, to test?
- If it is possible to update the structured data, and if the structured data
can be anything (array, string, boolean, dictionary), we might want an option
like: --json "..." to be able to set or update from the command line? The
current -k -v options seem to imply it must be a dict and only one level deep?
================
Comment at: lldb/include/lldb/API/SBBreakpointLocation.h:59
+ void SetScriptCallbackFunction(const char *callback_function_name,
+ SBStructuredData &extra_args);
+
----------------
shafik wrote:
> Could this be a `const &`
const doesn't mean anything for lldb::SB arguments. Since they contain shared
pointers or unique pointers, so just leave as is.
================
Comment at: lldb/include/lldb/API/SBBreakpointName.h:89
+ void SetScriptCallbackFunction(const char *callback_function_name,
+ SBStructuredData &extra_args);
+
----------------
shafik wrote:
> Could this be a `const &`
const doesn't mean anything for lldb::SB arguments. Since they contain shared
pointers or unique pointers, so just leave as is.
================
Comment at: lldb/include/lldb/Interpreter/OptionGroupPythonClassWithDict.h:26-30
OptionGroupPythonClassWithDict(const char *class_use,
+ bool is_class = true,
int class_option = 'C',
int key_option = 'k',
+ int value_option = 'v');
----------------
clang-format? Indent seems off (it was before too I see).
Repository:
rLLDB LLDB
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D68671/new/
https://reviews.llvm.org/D68671
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
