Change in binutils-gdb[master]: Change iterate_over_breakpoints to take a function_view

[Simon Marchi (Code Review)]

Simon Marchi has posted comments on this change.

Change URL: https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21
......................................................................


Patch Set 2: Code-Review-1

(6 comments)

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/breakpoint.c 
File gdb/breakpoint.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/breakpoint.c@15131 
PS2, Line 15131: iterate_over_breakpoints
               :   (gdb::function_view<bool (breakpoint *)> callback)
This would fit on a single line.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/dummy-frame.c 
File gdb/dummy-frame.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/dummy-frame.c@169 
PS2, Line 169:   iterate_over_breakpoints ([dummy] (breakpoint* bp)
Same comment about formatting as in py-finishbreakpoint.c.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/guile/scm-breakpoint.c 
File gdb/guile/scm-breakpoint.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/guile/scm-breakpoint.c@551 
PS2, Line 551: ([&] (breakpoint *bp) { return bpscm_build_bp_list(bp, &list); })
For readability, I'd like if you took that out of the "if".  Either:

  auto callback = [&] (breakpoint *bp)
    {
      return bpscm_build_bp_list(bp, &list);
    };
  if (iterate_over_breakpoints (callback) != NULL)
    return SCM_BOOL_F;

or

  breakpoint *bp_ret = iterate_over_breakpoints ([&] (breakpoint *bp)
    {
      return bpscm_build_bp_list(bp, &list);
    });

  if (bp_ret != NULL)
    return SCM_BOOL_F;

Oooooor, it seems to me like bpscm_build_bp_list can never return true, so iterate_over_breakpoints will never return a breakpoint.  So you could maybe get rid of this "if", that's indeed the case.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/python/py-breakpoint.c 
File gdb/python/py-breakpoint.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/python/py-breakpoint.c@909 
PS2, Line 909:       ([&] (breakpoint *bp) { return build_bp_list(bp, list.get ()); }) != NULL)
Same comment about formatting as in scm-breakpoint.o, except here the callback function can actually fail (in theory).


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/python/py-finishbreakpoint.c 
File gdb/python/py-finishbreakpoint.c:

https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/python/py-finishbreakpoint.c@386 
PS2, Line 386: iterate_over_breakpoints ([&] (breakpoint *bp) {
             : 			    return bpfinishpy_detect_out_scope_cb
             : 			      (bp, bs == NULL ? NULL : bs->breakpoint_at); });
I was told before to place the curly braces of the lambda like this, to mimic how we format scopes:

  iterate_over_breakpoints ([&] (breakpoint *bp)
    {
      return bpfinishpy_detect_out_scope_cb
        (bp, bs == NULL ? NULL : bs->breakpoint_at);
    });

I'd suggest to use that, I think it's quite readable.


https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21/2/gdb/python/py-finishbreakpoint.c@399 
PS2, Line 399:   iterate_over_breakpoints ([&] (breakpoint *bp) {
Same.



-- 
To view, visit https://gnutoolchain-gerrit.osci.io/r/c/binutils-gdb/+/21
To unsubscribe, or for help writing mail filters, visit https://gnutoolchain-gerrit.osci.io/r/settings