[PATCH] D28684: [Support/Compression] - Change zlib API to return Error instead of custom status.

[David Blaikie via llvm-commits]

On Mon, Jan 16, 2017 at 7:44 AM Rafael Avila de Espindola via llvm-commits <
llvm-commits at lists.llvm.org> wrote:

> George Rimar <grimar at accesssoftek.com> writes:
>
> >>> +  Error E = zlib::compress(StringRef(UncompressedNameStrings),
> >>> +                           CompressedNameStrings,
> zlib::BestSizeCompression);
> >>> +  if (E)
> >>>      return
> make_error<InstrProfError>(instrprof_error::compress_failed);
> >>
> >>This does't handle the error, does it? Do we have a test that covers it?
> >
> > I think it does. What do you think is wrong here ?
> > I am not sure about if we have test, but checking
> > if (Error) { ... }
> > seems to be a common practice I believe. Examples from llvm:
> >
> >  1)     Error E = CVTD.dump(Record, TDV);
> >       if (E) {
> >         logAllUnhandledErrors(std::move(E), errs(), "error: ");
>

^ this line handles the error. The InstrProfError example doesn't handle
the 'E' and will assert-fail if an error is produced from zlib::compress
because it would be unhandled.


> >         llvm_unreachable("produced malformed type record");
> >       }
> >
> > 2)   Error E = debugMiscompilation();
> >       if (!E)
> >         return Error::success();
> >
> > And I am pretty sure I debbugged that and other places when wrote it.
>
> If I remember correctly, an "if (E)" is sufficient when E doesn't
> actually contain an error, but if it contains an error E must be handled
> or propagated.
>

Yep.


>
> Cheers,
> Rafael
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170116/aa9f0742/attachment.html>