This is an archive of the discontinued LLVM Phabricator instance.

Fix two (three) more issues with unchecked Error.
ClosedPublic

Authored by srhines on Aug 14 2017, 10:17 PM.

Details

Summary

If assertions are disabled, but LLVM_ABI_BREAKING_CHANGES is enabled,
this will cause an issue with an unchecked Success. Switching to
consumeError() is the correct way to bypass the check. This patch also
includes disabling 2 tests that can't work without assertions enabled,
since llvm_unreachable() with NDEBUG won't crash.

Event Timeline

srhines created this revision.Aug 14 2017, 10:17 PM

https://reviews.llvm.org/D36728 is related for Clang (where this was discovered to crash clang-format in some configurations).

srhines updated this revision to Diff 111289.Aug 15 2017, 5:09 PM

Switch to cantFail

srhines added a subscriber: lhames.Aug 15 2017, 5:10 PM
lhames added inline comments.Aug 15 2017, 5:53 PM
include/llvm/ExecutionEngine/Orc/OrcRemoteTargetClient.h
171–174

Since we're not using the error we can avoid naming it at all and just have:

//FIXME: Add error poll.
cantFail(Client.deregisterEHFrames(Frame.Addr, Frame.Size));
srhines updated this revision to Diff 111411.Aug 16 2017, 1:28 PM

Remove unnecessary variable and assertion.

srhines marked an inline comment as done.Aug 16 2017, 1:28 PM

PTAL

lhames accepted this revision.Aug 24 2017, 11:56 AM

LGTM. :)

This revision is now accepted and ready to land.Aug 24 2017, 11:56 AM
This revision was automatically updated to reflect the committed changes.