This is an archive of the discontinued LLVM Phabricator instance.

Always evaluate the second argument for CHECK() lazily.
ClosedPublic

Authored by ruiu on Dec 6 2017, 12:40 PM.

Details

Event Timeline

ruiu created this revision.Dec 6 2017, 12:40 PM
pcc added a subscriber: pcc.Dec 6 2017, 1:37 PM
pcc added inline comments.
lld/include/lld/Common/ErrorHandler.h
117

Perhaps it would be better to support only the two-argument form via the CHECK macro? That would not only simplify the macro definition, but also if I look at a check call with a single argument, I can tell from the name that it is just a straightforward function call.

ruiu updated this revision to Diff 125798.Dec 6 2017, 1:37 PM
  • Use llvm::function_ref instead of std::function
ruiu added inline comments.Dec 6 2017, 1:41 PM
lld/include/lld/Common/ErrorHandler.h
117

Yeah that probably makes more sense. I thought that this macro isn't that bad, but we should avoid this sort of hack.

ruiu updated this revision to Diff 125804.Dec 6 2017, 1:52 PM
  • Use C macro only for the arity 2 check function.
pcc accepted this revision.Dec 6 2017, 2:03 PM

LGTM

This revision is now accepted and ready to land.Dec 6 2017, 2:03 PM
This revision was automatically updated to reflect the committed changes.