Author: Med Ismail Bennani Date: 2020-07-01T12:37:00+02:00 New Revision: 56bb1d1755ae38b4e7a67f775978b18a601f215f
URL: https://github.com/llvm/llvm-project/commit/56bb1d1755ae38b4e7a67f775978b18a601f215f DIFF: https://github.com/llvm/llvm-project/commit/56bb1d1755ae38b4e7a67f775978b18a601f215f.diff LOG: [lldb/api] Improve error reporting in SBBreakpoint::AddName (NFCI) 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.benn...@gmail.com> Added: Modified: lldb/bindings/interface/SBBreakpoint.i lldb/include/lldb/API/SBBreakpoint.h lldb/source/API/SBBreakpoint.cpp lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py Removed: ################################################################################ diff --git a/lldb/bindings/interface/SBBreakpoint.i b/lldb/bindings/interface/SBBreakpoint.i index 20354346be90..a2d747db0bf6 100644 --- a/lldb/bindings/interface/SBBreakpoint.i +++ b/lldb/bindings/interface/SBBreakpoint.i @@ -206,6 +206,9 @@ public: bool AddName (const char *new_name); + SBError + AddNameWithErrorHandling (const char *new_name); + void RemoveName (const char *name_to_remove); diff --git a/lldb/include/lldb/API/SBBreakpoint.h b/lldb/include/lldb/API/SBBreakpoint.h index 5a94297b888e..c9a52fcacf1a 100644 --- a/lldb/include/lldb/API/SBBreakpoint.h +++ b/lldb/include/lldb/API/SBBreakpoint.h @@ -105,6 +105,8 @@ class LLDB_API SBBreakpoint { bool AddName(const char *new_name); + SBError AddNameWithErrorHandling(const char *new_name); + void RemoveName(const char *name_to_remove); bool MatchesName(const char *name); diff --git a/lldb/source/API/SBBreakpoint.cpp b/lldb/source/API/SBBreakpoint.cpp index 283dd7ea8253..0b7bf3c4bd31 100644 --- a/lldb/source/API/SBBreakpoint.cpp +++ b/lldb/source/API/SBBreakpoint.cpp @@ -652,19 +652,28 @@ SBError SBBreakpoint::SetScriptCallbackBody(const char *callback_body_text) { bool SBBreakpoint::AddName(const char *new_name) { LLDB_RECORD_METHOD(bool, SBBreakpoint, AddName, (const char *), new_name); + SBError status = AddNameWithErrorHandling(new_name); + return status.Success(); +} + +SBError SBBreakpoint::AddNameWithErrorHandling(const char *new_name) { + LLDB_RECORD_METHOD(SBError, SBBreakpoint, AddNameWithErrorHandling, + (const char *), new_name); + BreakpointSP bkpt_sp = GetSP(); + SBError status; if (bkpt_sp) { std::lock_guard<std::recursive_mutex> guard( bkpt_sp->GetTarget().GetAPIMutex()); - Status error; // Think I'm just going to swallow the error here, it's - // probably more annoying to have to provide it. + Status error; bkpt_sp->GetTarget().AddNameToBreakpoint(bkpt_sp, new_name, error); - if (error.Fail()) - return false; + status.SetError(error); + } else { + status.SetErrorString("invalid breakpoint"); } - return true; + return status; } void SBBreakpoint::RemoveName(const char *name_to_remove) { @@ -1015,6 +1024,8 @@ void RegisterMethods<SBBreakpoint>(Registry &R) { LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, SetScriptCallbackBody, (const char *)); LLDB_REGISTER_METHOD(bool, SBBreakpoint, AddName, (const char *)); + LLDB_REGISTER_METHOD(lldb::SBError, SBBreakpoint, AddNameWithErrorHandling, + (const char *)); LLDB_REGISTER_METHOD(void, SBBreakpoint, RemoveName, (const char *)); LLDB_REGISTER_METHOD(bool, SBBreakpoint, MatchesName, (const char *)); LLDB_REGISTER_METHOD(void, SBBreakpoint, GetNames, (lldb::SBStringList &)); diff --git a/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py b/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py index e421140e0182..821b7c809caf 100644 --- a/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py +++ b/lldb/test/API/functionalities/breakpoint/breakpoint_names/TestBreakpointNames.py @@ -99,8 +99,8 @@ def do_check_names(self): other_bkpt_name = "_AnotherBreakpoint" # Add a name and make sure we match it: - success = bkpt.AddName(bkpt_name) - self.assertTrue(success, "We couldn't add a legal name to a breakpoint.") + success = bkpt.AddNameWithErrorHandling(bkpt_name) + self.assertSuccess(success, "We couldn't add a legal name to a breakpoint.") matches = bkpt.MatchesName(bkpt_name) self.assertTrue(matches, "We didn't match the name we just set") @@ -113,7 +113,7 @@ def do_check_names(self): self.check_name_in_target(bkpt_name) # Add another name, make sure that works too: - bkpt.AddName(other_bkpt_name) + bkpt.AddNameWithErrorHandling(other_bkpt_name) matches = bkpt.MatchesName(bkpt_name) self.assertTrue(matches, "Adding a name means we didn't match the name we just set") @@ -142,8 +142,8 @@ def do_check_illegal_names(self): "CantHave-ADash", "Cant Have Spaces"] for bad_name in bad_names: - success = bkpt.AddName(bad_name) - self.assertTrue(not success,"We allowed an illegal name: %s"%(bad_name)) + success = bkpt.AddNameWithErrorHandling(bad_name) + self.assertTrue(success.Fail(),"We allowed an illegal name: %s"%(bad_name)) bp_name = lldb.SBBreakpointName(self.target, bad_name) self.assertFalse(bp_name.IsValid(), "We made a breakpoint name with an illegal name: %s"%(bad_name)); @@ -164,8 +164,8 @@ def do_check_using_names(self): other_bkpt_name= "_AnotherBreakpoint" # Add a name and make sure we match it: - success = bkpt.AddName(bkpt_name) - self.assertTrue(success, "We couldn't add a legal name to a breakpoint.") + success = bkpt.AddNameWithErrorHandling(bkpt_name) + self.assertSuccess(success, "We couldn't add a legal name to a breakpoint.") bkpts = lldb.SBBreakpointList(self.target) self.target.FindBreakpointsByName(bkpt_name, bkpts) @@ -243,8 +243,8 @@ def do_check_configuring_names(self): # Now add this name to a breakpoint, and make sure it gets configured properly bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10) - success = bkpt.AddName(self.bp_name_string) - self.assertTrue(success, "Couldn't add this name to the breakpoint") + success = bkpt.AddNameWithErrorHandling(self.bp_name_string) + self.assertSuccess(success, "Couldn't add this name to the breakpoint") self.check_option_values(bkpt) # Now make a name from this breakpoint, and make sure the new name is properly configured: @@ -317,8 +317,8 @@ def check_permission_results(self, bp_name): unprotected_bkpt = self.target.BreakpointCreateByLocation(self.main_file_spec, 10) unprotected_id = unprotected_bkpt.GetID() - success = protected_bkpt.AddName(self.bp_name_string) - self.assertTrue(success, "Couldn't add this name to the breakpoint") + success = protected_bkpt.AddNameWithErrorHandling(self.bp_name_string) + self.assertSuccess(success, "Couldn't add this name to the breakpoint") self.target.DisableAllBreakpoints() self.assertEqual(protected_bkpt.IsEnabled(), True, "Didnt' keep breakpoint from being disabled") _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits