[PATCH] D66011: [llvm-readobj] - Remove 'error(Error EC)' helper.

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Aug 9 06:25:04 PDT 2019


MaskRay added inline comments.


================
Comment at: tools/llvm-readobj/llvm-readobj.cpp:385
 
 void reportError(StringRef Input, Error Err) {
+  if (!Err)
----------------
jhenderson wrote:
> grimar wrote:
> > MaskRay wrote:
> > > reportError now checks `if (!Err)`. Is it still appropriate to call it `reportError`? I want to hear other opinions.
> > Previously `error(Error EC)` did that. I can probably remove `if (!Err)` check to avoid the confusion.
> I'd probably replace the `if (!Err)` check with an equivalent assertion. I think it's natural to write code like:
> 
> ```
> if (Error E = doSomething())
>   reportError(File, std::move(E));
> ```
> 
> whereas the following looks weird to me:
> 
> ```
> reportError(doSomethingWhichWillSucceed());
> ```
> 
> An alternative name might be "checkError" if you didn't do that.
`checkError` looks good to me.


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

https://reviews.llvm.org/D66011





More information about the llvm-commits mailing list