[Lldb-commits] [lldb] [lldb] Change the implementation of Status to store an llvm::Error (NFC) (PR #106774)

[Adrian Prantl via lldb-commits]

adrian-prantl wrote:

> Two high-level comments:
> 
> * I think that the `FromError`/`clone` changes would be better of as a separate patch(es). I have no problem with the changes themselves, but I think it'd be better to separate the meat of this patch from the mechanical renaming. The renames could be done either before an after the patch, as (AFAICT) the functions are just fancy names for (copy) constructors.

I can do that.

> * I think this class should accept an open class hierarchy (like llvm::Error) does and not assume that it can enumerate all the error subclasses. I think the only additional property that we need of the error classes is the ability to clone themselves, which we could achieve by providing a `ClonableErrorInfo` class with an abstract `clone` method. We can still keep the code which converts all unclonable errors into a string error (but I'd only do it after a copy, not immediately during construction). The thing I want to avoid is the proliferation of very specific error types inside the base Status class -- the mach/windows error types should arguably be defined in the host library, and even the ExpressionError type might be better off inside the command interpreter or something. I'm not saying you have to do the refactor now, but I'd like to have the option to do so in the future.

It really looks like the need for cloneable errors and long-term storage of errors is the key insight that came out of this patch. I'll think a bit about this as I'm splitting this patch up further.

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