[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