[Lldb-commits] [lldb] Add doc strings for SBValue::IsValid and SBValue::GetError. (PR #94007)

[Alex Langford via lldb-commits]

================
@@ -45,6 +45,36 @@ SBValue instance which interprets the value object as representing the head of a
 linked list."
 ) lldb::SBValue;
 
+%feature("docstring", "
+Returns true if the SBValue holds any useful state
+and false otherwise.
+IsValid is a very weak API, lldb will only return
+invalid SBValues if it has no useful information
+about the SBValue.
+The two main ways you will end up with an invalid
+SBValue are:
+1) default constructed SBValues are not valid.
+2) SBValues that have outlived their SBTarget are
+no longer valid since its not safe to ask them
----------------
bulbazord wrote:

I think I understand what you're saying better after re-reading it a few times. I formed a different meaning conceptually the first time I read it.

The two scenarios both have the exact same behavior. Why talk about safety in the second scenario but not the first? Rewording it a bit, it sounds like this: "Since it is not safe to ask them questions, SBValues that have outlived their SBTargets are invalid." Once the SBTarget dies and the SBValue is invalidated, it should behave exactly like the default constructed.

I don't understand the use of "safe" here. Safety implies that a risk exists, and I just don't see what could go wrong. The worst thing that could happen is you get back an invalid/empty object, and that's no different from asking a default constructed SBValue questions. LLDB shouldn't be crashing, shouldn't be silently corrupting or mutating state behind your back, and shouldn't be giving you valid-looking values that aren't valid when you ask a question of an invalid SBValue.

The second sentence didn't feel like it followed naturally from the first because of that. It describes something I would consider to be innocuous, so it didn't immediately click for me. Also, the subject of the sentence being lldb is too general. SBValue makes more sense since the target audience is developers trying to use SBValue. I was also confused at the use of "instead". I initially tried to interpret the sentence as describing an alternative to asking the questions, not an alternative to a behavior where it returned something other than a default constructed object.

My suggestion would be to write something like this:
"Calling methods on an invalid SBValue will return default constructed objects.
The two main ways you will end up with an invalid SBValue are:
1.) default constructed SBValues are not valid.
2.) SBValues that have outlived their SBTarget are
 no longer valid.
 
You can distinguish between an invalid SBValue and a valid SBValue with some missing data using IsValid when an SBValue returns a default constructed object from one of its methods."

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