medismailben wrote: > > I think it would be better to change `Process::ReadMemoryRanges` to return > > `llvm::SmallVector<llvm::Expected<llvm::MutableArrayRef<uint8_t>>>` and > > then we could change `lldb_private::StringList` to hold a > > `std::vector<llvm::Expected<std::string>>`, add a `HasError` method to > > check if it contains any error, as well as a `GetErrorAtIndex()` method to > > surface the error appropriately. > > This was discussed during the RFC, and the conclusion was that if a memory > read did not return the number of bytes requested, a user could call the > single-address variant if they really wanted more details. Even then, it was > felt that _what_ the error was was not relevant, only the fact that _some_ > error occurred, which is already expressed by the read length. >
I saw that in the code ... we can fix debug server & lldb_private to surface a relevant error message to the user but we can't modify the API after it lands. That's why I'm so picky about this. I'd rather have that room to improve in the future than having to make a new API and make it confusing which one to use. > > That way you could keep your SBStringString api, but in the even where > > there was a memory read failure for one of the addresses, the user could > > get the error from the same SBStringList object. > > This would be akin to changing the behavior of an SB object, which I don't > think we are allowed to do I don't think changing an SB object behavior is forbidden as long as the API don't change. Also, I don't think my suggestion would change the behavior of `SBStringList` since it'd be purely additive: Let's say you have a memory read error at index 2 with a 4 elements `SBStringList` object, `SBStringList.GetStringAtIndex(2)` would still give you an empty string, which is what you're doing currently, but you could get the error from a different `SBStringList.GetErrorAtIndex(2)` API. Similarly, if there is no error at that index, that API will return a default construct `lldb.SBError`: ``` >>> err = lldb.SBError() >>> err.Success() True ``` > I will just remove the SBAPI portion of this PR. This will give us time to > think about the public API. Sounds good, may be @jingham or @JDevlieghere will have some ideas how to deal with this, after the holidays. https://github.com/llvm/llvm-project/pull/172026 _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
