labath added inline comments.
================
Comment at: lldb/bindings/lua/lua-wrapper.swig:21-22
+{
+ lldb::SBFrame sb_frame(stop_frame_sp);
+ lldb::SBBreakpointLocation sb_bp_loc(bp_loc_sp);
+
----------------
tammela wrote:
> labath wrote:
> > What's up with the copying? Is it to ensure the callback does not modify
> > the caller's values? If so, you could just drop the `&` from the signature,
> > and have the compiler copy these for you...
> `SBFrame` and `SBBreakpointLocation` only provide copy constructors. I can't
> see the difference between the two strategies, could you elaborate?
Ok, let's try this.
My main point was that a function which receives an argument as a (non-const)
reference, and then does not modify the referenced object is weird. One just
doesn't do that. It should either take a const reference or a plain value.
Using a plain value is simpler, particularly in cases where you need to locally
modify the object you received, but you don't want the modifications to affect
the callers value. That's because in this case the compiler will do the copying
for you:
```
void foo(SBFrame foo, SBFrame &bar, const SBFrame &baz) {
// I don't have to worry about modifying foo, because it's my local copy
foo = ...;
// Modifications to bar will be visible in the caller
bar = ...;
// I can't modify baz. If I really need to do that, I need to make a local
copy
SBFrame baz_copy = baz;
baz_copy = ...;
}
```
================
Comment at:
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test:10
+run
+# CHECK: frame #0
+# CHECK: Process {{[0-9]+}} exited with status = 0
----------------
"frame #0" will also get printed if the process stops for any reason. I'd
suggest printing a completely random string instead (`print("bacon")`, for
example)
================
Comment at:
lldb/test/Shell/ScriptInterpreter/Lua/breakpoint_oneline_callback.test:5
+b main
+breakpoint command add -s lua -o 'print(frame)'
+run
----------------
tammela wrote:
> labath wrote:
> > Could we also have a test where calling the callback raises an error? And
> > for the different values of the "stop" argument that can be returned from
> > the callback? And when compiling the callback expression results in an
> > error?
> Apparently when the user sets the breakpoint and it fails to compile the code
> lldb will not report any errors and it will fail silently. I will address
> this particular test case in another patch since it affects Python as well.
Sounds good.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D91508/new/
https://reviews.llvm.org/D91508
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
