This is an archive of the discontinued LLVM Phabricator instance.

Change checked arithmetic functions API to return Optional
ClosedPublic

Authored by george.karpenkov on Jun 13 2018, 10:41 AM.

Details

Summary

Returning optional is much safer.
The previous API had potential to cause use of undefined variables, if the value passed by pointer was accidentally read afterwards.

Diff Detail

Event Timeline

NoQ accepted this revision.Jun 13 2018, 11:14 AM
This revision is now accepted and ready to land.Jun 13 2018, 11:14 AM
This revision was automatically updated to reflect the committed changes.
dblaikie added inline comments.
llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp
41 ↗(On Diff #151212)

You might be able to write this and other non-None values as the bare integer, rather than explicitly wrapping it in Optional? (there should be an implicit conversion, I'd think) Or does something about the gtest framework get in the way of that?

llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp
41 ↗(On Diff #151212)

@dblaikie Hm I'm not sure whether it would work, EXPECT_EQ is not explicitly marked to expect a specific optional as a second parameter.
I wanted to have an explicit version here, but if implicit works as well I would not mind the change.