[Lldb-commits] [lldb] [lldb] All ValueObjectSP instances are now valid (non-null) but have an error state (In-progress | Take 2) (PR #77375)

[via lldb-commits]

jimingham wrote:



> On Jan 9, 2024, at 12:29 PM, Pete Lawrence ***@***.***> wrote:
> 
> 
> I don't understand why step 2 is necessary; what does the optional actually add? Is there a useful distinction between "I got an optional that was empty" and "I got a ValueObjectSP with an error"? I didn't see any places where you use that distinction.
> 
> Returning the optional is also awkward because we either have to check that the optional had a value and THEN check that GetError().Success() returns true, which is awkward, or we have to add the rule that if an optional return is not empty, then the ValueObjectSP is not in the error state, which is also a bit odd.
> 
> Step 2 is necessary now that the new sub-class design doesn't allow for null shared pointers. Returning an optional is now the (remaining) best option to for a function/method to say "I have nothing to return to you, but it's not an error" in its return value.
> 
> The vast majority of places that return an error inside the Value Object return ValueObjectSP, not an optional of one.
> 
> I agree it's awkward, but you can think of it as a different way to represent the same awkwardness that's already been there with the existing tri-state approach:
> 
> valueObjectSP == nullptr
> valueObjectSP != nullptr && valueObjectSP->Fail()
> valueObjectSP != nullptr && valueObjectSP->Success()
In my mind, the point of this change is that I NEVER get empty ValueObjectSP, we only have real ValueObject's in them, and use the error state of the VO to convey what the "returning an empty VO" did before.

How many places care, when they are returned an empty value object sp, about the particular nature of the "why I couldn't give this to you" beyond what the error message itself would convey?  If we everywhere treat "no returned ValueObject" the same way they would treat "a ValueObject returned with GetError().Fail() true" then one of these tristate was in fact not expressing an independent value, and we don't need to preserve it.

If there really are some places where we need to behave differently between the case where "You asked me for a child that doesn't exist" and where "You asked me for a child that does exist but I couldn't make it for some reason" we can add an ErrorType like "ValueObjectNotFound" and use that to programmatically distinguish these cases.  But I bet for the most part if you ask for a VO and don't get one, you're just going to report the error.

Jim

> —
> Reply to this email directly, view it on GitHub <https://github.com/llvm/llvm-project/pull/77375#issuecomment-1883741842>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ADUPVW77VPSUL4PURLAM6MDYNWSEJAVCNFSM6AAAAABBSCJVSGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQOBTG42DCOBUGI>.
> You are receiving this because you are on a team that was mentioned.
> 



https://github.com/llvm/llvm-project/pull/77375