https://github.com/labath commented:
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 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. https://github.com/llvm/llvm-project/pull/106774 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
