[PATCH] D72176: Make ErrorList class default constructible and add simple push_back() method

[Lang Hames via Phabricator via llvm-commits]

lhames added a comment.

Actually, since I'm pontificating about Error on this thread anyway, the other big change that 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:

Instead of:

  Error foo();
  
  handleErrors(foo(),
               [](ErrType1 &E1) {
                 ...
               },
               [](ErrType2 &E2) {
                 ...
               },
               ...);

We could have:

  auto Err = foo();
  if (auto E1 = err_cast<ErrType1>(Err)) {
    ..
  } else if (auto E2 = err_cast<ErrType2>(Err)) {
    ...
  }

There are pros and cons to each scheme:
handleErrors -- Pros: Failure value names (E1, E2) scoped to handler only. Cons: Not intuitive -- you need to read the docs to figure out how to use it. Can't be used in C API.
err_cast -- Pros: Intuitive, and C API implementation would be a straightforward mapping. Cons: Failure value names leak into else-if scopes.

On balance I think err_cast is a better answer. But we need to remove (or at least demote) ErrorList before we could switch.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D72176/new/

https://reviews.llvm.org/D72176