[libc-commits] [PATCH] D106086: [libc] Add a new test matcher for tests raising floating point exceptions.

[Siva Chandra via Phabricator via libc-commits]

sivachandra added inline comments.


================
Comment at: libc/utils/FPUtil/TestHelpers.cpp:98
+  auto oldSIGFPEHandler = signal(SIGFPE, &sigfpeHandler);
+  std::unique_ptr<FunctionCaller> funcUP(func);
+
----------------
mcgrathr wrote:
> Just make unique_ptr the argument type so callers construct it in place.
We cannot use `std::unique_ptr` in the API as we will then have to include `<memory>` in the header file which will lead to header-mixups in the unit test. That is why I have arranged it this way. Added a comment now in the header file about "ownership transfer".


================
Comment at: libc/utils/FPUtil/TestHelpers.h:79
+      Func f;
+      explicit Callable(Func f) : f(f) {}
+      void call() override { f(); }
----------------
mcgrathr wrote:
> This ctor is superfluous if you just construct with `Callable{func}`.
> 
My understanding about brace initializers could be wrong or incomplete here but since the class is dynamic (because of the virtual methods), its objects cannot be initialized with `{...}`? Clang infact complains that there is no appropriate constructor for it when I use the brace initializer.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D106086/new/

https://reviews.llvm.org/D106086