[PATCH] D72176: Make ErrorList class default constructible and add simple push_back() method
Stefan Gränitz via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Sat Feb 8 03:53:42 PST 2020
sgraenitz added a comment.
In D72176#1834446 <https://reviews.llvm.org/D72176#1834446>, @lhames wrote:
> This seems like a nice change for ErrorList.
>
> What are you using it for though?
> [...]
> In my experience, whenever I have used ErrorList it was really because I wanted to be able to report multiple errors, never because I wanted to be able to recover from them. However I think this kind of reporting is better handled by a logging idiom (like ExecutionSession::reportError).
Yes, basically I'd use it for reporting errors and indeed it's better done like logging. At first, I like how ErrorList packed more errors in the Payload to make a list, that seemed very elegant to me. But you are right, if it adds unnecessary complexity to the primary usage, it probably not the right way to go.
> I would like to see (which is also impacted by ErrorList) would be to ditch handleErrors entirely. If we had the "one Error/Expected == one failure" invariant, we could switch to an "err_cast" idiom
That seems like a good idea. Not sure if I ever saw a handleErrors with more than one handler in production code. I agree that if we have to drop ErrorList for it, we should do so and not make it more convenient to use.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D72176/new/
https://reviews.llvm.org/D72176
More information about the llvm-commits
mailing list