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

George Rimar via llvm-commits llvm-commits at lists.llvm.org
Tue Jan 17 02:02:48 PST 2017


>>>> +  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: ");
>>         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.
>
>Cheers,
>Rafael

I see now, thanks. Checked state of Error object 
verified only when LLVM_ENABLE_ABI_BREAKING_CHECKS defined. It was not for me.
That is why code worked for me during debugging.

George.


More information about the llvm-commits mailing list