https://github.com/labath commented:
Okay, at this I don't think I have any more comments about functionality. The remaining comments are either stylistic (which should be self-explanatory) or about error handling (which I'm going to write about here). For the error handling, I missed the fact that your new implementation throws away the structured diagnostic info after using it to construct the error string. That kinda misses the point, which is to provide the rest of lldb with a structured representation of the error. It looks like the base DiagnosticError is abstract, so for that we need to create our own error subclass of that. I tried to sketch it out in the inline comments (by combining the existing `ExpressionError` class and your code), but this is only a rough sketch -- you'll certainly need to adjust it to make it work. You can tell whether you've succeded by looking at how the new "frame var" error messages are formatted. They should come out in the same was as in the snippet that [Jim posted](https://github.com/llvm/llvm-project/pull/120971#issuecomment-2722987750). The other set of suggested changes is about changing the error representation from Status to llvm::Error. We're basically already using the error object as if it was an llvm::Error, so I think we should just complete the transition and use it throughout the DIL. https://github.com/llvm/llvm-project/pull/120971 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
