Page MenuHomePhabricator

[lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI)
ClosedPublic

Authored by mib on Jun 30 2020, 7:49 AM.

Details

Summary

This patch improves the error reporting for SBBreakpoint::AddName by
adding a new method SBBreakpoint::AddNameWithErrorHandling that returns
a SBError instead of a boolean.

This way, if the breakpoint naming failed in the backend, the client
(i.e. Xcode), will be able to report the reason of that failure to the
user.

rdar://64765461

Signed-off-by: Med Ismail Bennani <medismail.bennani@gmail.com>

Event Timeline

mib created this revision.Jun 30 2020, 7:49 AM

We can't break the SB API, it's unfortunate but this should be an overload instead...

mib updated this revision to Diff 274561.Jun 30 2020, 11:49 AM
mib edited the summary of this revision. (Show Details)

Introduced SBBreakpoint::AddNameWithError to avoid API stability breakage.

JDevlieghere added inline comments.Jun 30 2020, 11:57 AM
lldb/bindings/interface/SBBreakpoint.i
210

In SBDebugger we have InitializeWithErrorHandling, maybe use the same suffix here for consistency?

lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
103

Seems like a good use case for Pavel's new assertSuccess helper? (https://reviews.llvm.org/D82759)

mib updated this revision to Diff 274633.Jun 30 2020, 3:44 PM
mib marked 2 inline comments as done.
  • Use new assertSuccess method in test.
  • Rename the method to AddNameWithErrorHandling for consistency.
mib edited the summary of this revision. (Show Details)Jun 30 2020, 3:47 PM
teemperor accepted this revision.Jul 1 2020, 3:23 AM

LGTM with some minor nits.

lldb/include/lldb/API/SBBreakpoint.h
108 ↗(On Diff #274633)

I wish we had some documentation when adding a name could fail (e.g. empty name string?) But we can also add this all later as most of this stuff needs documentation.

lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py
146 ↗(On Diff #274633)

I think we usually put spaces around operators/string literals.

This revision is now accepted and ready to land.Jul 1 2020, 3:23 AM
This revision was automatically updated to reflect the committed changes.
mib marked an inline comment as done.