openPMD / openPMD-api

:floppy_disk: C++ & Python API for Scientific I/O
https://openpmd-api.readthedocs.io
GNU Lesser General Public License v3.0
134 stars 51 forks source link

Fix `unique_ptr<T, Del>` constructor of UniquePtrWithLambda #1552

Closed franzpoeschel closed 7 months ago

franzpoeschel commented 8 months ago

I forgot adding tests for that constructor and it turns out it is broken. It's the most generic conversion constructor that the class has (there is a specialization for std::unique_ptr<T>), so this is not used very often. There are two problems:

  1. The implementation calls get_deleter() twice. This PR removes the second invocation.
  2. std::function needs a copyable lambda, but Del might be non-copyable. Add a custom path for that case.

This PR adds two tests for each case. Without this PRs, the compile time errors are as follows:

1:

In file included from /.../openPMD-api/include/openPMD/auxiliary/TypeTraits.hpp:24,                                                
                 from /.../openPMD-api/include/openPMD/Datatype.hpp:23,                                                            
                 from /.../openPMD-api/test/SerialIOTest.cpp:2:                                                                    
/.../openPMD-api/include/openPMD/auxiliary/UniquePtr.hpp: In instantiation of 'openPMD::UniquePtrWithLambda<T>::UniquePtrWithLambda(std::unique_ptr<T, Del>) [with Del = detail::CopyableDeleter<int>; T = int]':
/.../openPMD-api/test/SerialIOTest.cpp:705:71:   required from here
/.../openPMD-api/include/openPMD/auxiliary/UniquePtr.hpp:151:27: error: 'const struct detail::CopyableDeleter<int>' has no member named 'get_deleter'
  151 |                   deleter.get_deleter()(del_ptr);

2:

In file included from /nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/functional:59,
                 from /.../openPMD-api/include/openPMD/auxiliary/UniquePtr.hpp:3,
                 from /.../openPMD-api/include/openPMD/auxiliary/TypeTraits.hpp:24,
                 from /.../openPMD-api/include/openPMD/Datatype.hpp:23,
                 from /.../openPMD-api/test/SerialIOTest.cpp:2:
/nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/std_function.h: In instantiation of 'std::function<_Res(_ArgTypes ...)>::function(_Functor&&) [with _Functor = openPMD::UniquePtrWithLambda<int>::UniquePtrWithLambda<detail::NonCopyableDeleter<int> >(std::unique_ptr<int, detail::NonCopyableDeleter<int> >)::<lambda(openPMD::UniquePtrWithLambda<int>::T_decayed*)>; _Constraints = void; _Res = void; _ArgTypes = {int*}]':
/.../openPMD-api/include/openPMD/auxiliary/UniquePtr.hpp:143:17:   required from 'openPMD::UniquePtrWithLambda<T>::UniquePtrWithLambda(std::unique_ptr<T, Del>) [with Del = detail::NonCopyableDeleter<int>; T = int]'
/.../openPMD-api/test/SerialIOTest.cpp:728:74:   required from here
/nix/store/b7hvml0m3qmqraz1022fwvyyg6fc1vdy-gcc-12.2.0/include/c++/12.2.0/bits/std_function.h:439:69: error: static assertion failed: std::function target must be copy-constructible
  439 |           static_assert(is_copy_constructible<__decay_t<_Functor>>::value,
franzpoeschel commented 7 months ago

clang-format decided to make no captives today https://github.com/openPMD/openPMD-api/pull/1552/commits/68918b200e9819c2bb4771261d7f0af4d0202adb