https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/170862
>From afe71b2e714be44515fd31b679ca92e240aab40f Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 5 Dec 2025 14:07:23 +0000 Subject: [PATCH 1/2] [lldb] Document the behaviour of IsValid for SBError This reverts commit d20d84fec5945fcc16aa6f63879e1458d4af9ea6. In which I changed an SBError use so that when the function succeeded, IsValid on the SBError would be true. This seemed to make sense but SBError acts differently to other SB classes in this respect. For something like SBMemoryRegionInfo, if IsValid() is false, you can't do anything with it. However for SBError, IsValid() true only means there's some underlying error object in there. If the SBError represents a success, there's no need to put anything in there. You can see this intent from a lot of its methods, they all have handling for IsValid() false. This is not a bug but a misunderstanding of what IsValid means. Yes it does function the way I expected for most classes, but it does not for SBError and though that's not intuitive, it is consistent with how we describe IsValid in the documentation. So instead of changing that method's use of SBError I'm documenting this initially counterintuitive behaviour. --- lldb/docs/resources/sbapi.rst | 10 ++++++++++ lldb/include/lldb/API/SBError.h | 9 +++++++++ lldb/source/API/SBDebugger.cpp | 2 +- lldb/unittests/DAP/TestBase.cpp | 1 - 4 files changed, 20 insertions(+), 2 deletions(-) diff --git a/lldb/docs/resources/sbapi.rst b/lldb/docs/resources/sbapi.rst index 4ca3909e0f291..db596ddc9fbab 100644 --- a/lldb/docs/resources/sbapi.rst +++ b/lldb/docs/resources/sbapi.rst @@ -46,6 +46,16 @@ prepared to handle their opaque implementation pointer being empty, and doing something reasonable. We also always have an "IsValid" method on all the SB classes to report whether the object is empty or not. +.. note:: The implication of an object being "empty" can vary by class. + + For most classes, the lack of anything backing the class means that it + would not be valid to interact with it by calling any other methods + on it. + + One exception to this is ``SBError``, which can provide valid + information even when empty. This is because it does not need an + underlying object to be able to represent a success state. + Another piece of the SB API infrastructure is the Python (or other script interpreter) customization. SWIG allows you to add property access, iterators and documentation to classes. We place the property accessors and iterators in diff --git a/lldb/include/lldb/API/SBError.h b/lldb/include/lldb/API/SBError.h index 6a4a39c4db314..dd8c0f939775f 100644 --- a/lldb/include/lldb/API/SBError.h +++ b/lldb/include/lldb/API/SBError.h @@ -72,6 +72,15 @@ class LLDB_API SBError { explicit operator bool() const; + /// \brief Returns \c true if this object contains an underlying \c Status + /// object. + /// + /// That object may represent a success or a failure. When \c IsValid returns + /// \c false, it may be the case that the \c SBError represents a success but + /// does not contain a \c Status representing that success. + /// + /// It is safe to call \c Success or \c Fail in the case where \c IsValid + /// returns \c false. bool IsValid() const; bool GetDescription(lldb::SBStream &description); diff --git a/lldb/source/API/SBDebugger.cpp b/lldb/source/API/SBDebugger.cpp index f939955ba57c8..7a4bebfdf998e 100644 --- a/lldb/source/API/SBDebugger.cpp +++ b/lldb/source/API/SBDebugger.cpp @@ -179,7 +179,7 @@ void SBDebugger::Initialize() { lldb::SBError SBDebugger::InitializeWithErrorHandling() { LLDB_INSTRUMENT(); - SBError error((Status())); + SBError error; if (auto e = g_debugger_lifetime->Initialize( std::make_unique<SystemInitializerFull>())) { error.SetError(Status::FromError(std::move(e))); diff --git a/lldb/unittests/DAP/TestBase.cpp b/lldb/unittests/DAP/TestBase.cpp index f4dde9559e9d3..8cb459964f7d8 100644 --- a/lldb/unittests/DAP/TestBase.cpp +++ b/lldb/unittests/DAP/TestBase.cpp @@ -72,7 +72,6 @@ void DAPTestBase::TearDown() { void DAPTestBase::SetUpTestSuite() { lldb::SBError error = SBDebugger::InitializeWithErrorHandling(); - EXPECT_TRUE(error.IsValid()); EXPECT_TRUE(error.Success()); } void DAPTestBase::TeatUpTestSuite() { SBDebugger::Terminate(); } >From 57fa56dacd02ade23c4611eceeac6e4f3d47689a Mon Sep 17 00:00:00 2001 From: David Spickett <[email protected]> Date: Fri, 5 Dec 2025 14:45:52 +0000 Subject: [PATCH 2/2] formatting --- lldb/docs/resources/sbapi.rst | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lldb/docs/resources/sbapi.rst b/lldb/docs/resources/sbapi.rst index db596ddc9fbab..8c15e371a5012 100644 --- a/lldb/docs/resources/sbapi.rst +++ b/lldb/docs/resources/sbapi.rst @@ -46,15 +46,16 @@ prepared to handle their opaque implementation pointer being empty, and doing something reasonable. We also always have an "IsValid" method on all the SB classes to report whether the object is empty or not. -.. note:: The implication of an object being "empty" can vary by class. +.. note:: + The implication of an object being "empty" can vary by class. - For most classes, the lack of anything backing the class means that it - would not be valid to interact with it by calling any other methods - on it. + For most classes, the lack of anything backing the class means that it + would not be valid to interact with it by calling any other methods + on it. - One exception to this is ``SBError``, which can provide valid - information even when empty. This is because it does not need an - underlying object to be able to represent a success state. + One exception to this is ``SBError``, which can provide valid + information even when empty. This is because it does not need an + underlying object to be able to represent a success state. Another piece of the SB API infrastructure is the Python (or other script interpreter) customization. SWIG allows you to add property access, iterators _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
