[PATCH] D48137: Change checked arithmetic functions API to return Optional

[George Karpenkov via Phabricator via llvm-commits]

george.karpenkov added inline comments.


================
Comment at: llvm/trunk/unittests/Support/CheckedArithmeticTest.cpp:41
+  EXPECT_EQ(checkedMul<int16_t>(Min, 2), None);
+  EXPECT_EQ(checkedMul<int16_t>(10, 2), Optional<int16_t>(20));
 }
----------------
dblaikie wrote:
> 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?
@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.


Repository:
  rL LLVM

https://reviews.llvm.org/D48137