[Lldb-commits] [lldb] [lldb] Change ValueObject::AddressOf() to return Expected (NFC) (PR #106831)

[via lldb-commits]

jimingham wrote:

I don't think this is the right way to handle ValueObjects in particular.  Since ValueObjects carry their errors along with them, there's really no good excuse ever to return an empty ValueObjectSP.  We do that in some places, but it really makes the interface awkward: currently you have to check first whether you got a ValueObjectSP, then you have to check it's error to see if you can use it.

In particular, AddressOf should never be returning an empty ValueObjectSP, since there's always an interesting reason why this failed if it did.  If anything we should be asserting at the end of AddressOf if somebody is trying to get away with returning an empty ValueObjectSP.  Adding an Expected makes is seem legit to return an empty ValueObjectSP, so that seems like going in the wrong direction.

If we are going to move in the direction of making ValueObject's in particular more secure to use, we should make it so that there are no more interfaces that return ValueObjectSP's, just ValueObjects.  You can always get the SP if you need it since ValueObject inherits from shared_from_this, so that shouldn't be a problem.

That way we'll always be ensured that if producing a ValueObject fails, we'll always have a reason.

So I don't think that wrapping Expected around ValueObjectSP's is the right way to go here.

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