TileDB
TileDB copied to clipboard
Remove serialization non C.41 constructors from fragment_metadata_from_capnp
This pull request has been linked to Shortcut Story #40131: Remove serialization non C.41 constructors from fragment_metadata_from_capnp..
@teo-tsirpanis let's not work more on this PR. I started to review and it became obvious to me we need to design something different here. The 30 parameters constructor doesn't look good.
Undrafting, regardless of whether we will take it, it is finished and ready for review.
I restored some move constructors and am getting errors like static_assert failed: 'T must be constructible from either (allocator_arg_t, const Alloc&, Types...) or (Types..., const Alloc&) if uses_allocator_v<remove_cv_t<T>, Alloc> is true'
. Any idea on how to fix them?
I restored some move constructors and am getting errors like
static_assert failed: 'T must be constructible from either (allocator_arg_t, const Alloc&, Types...) or (Types..., const Alloc&) if uses_allocator_v<remove_cv_t<T>, Alloc> is true'
. Any idea on how to fix them?
@davisp any ideas here?
That’s a rule for constructors on PMR containers stored inside other pmr containers. Either the constructor has to have the last argument for the Allocator, or the first argument must be the special trait marker and the second argument is the allocator argument. So likely this was adding the move constructor without providing an argument to accept the allocator.
Pablo Halpern covers the container constructor stuff during this part of his cppcon talk: https://youtu.be/v3dz-AKOVL8?t=2360&si=qKn94l2h8x9uwtHT
Thanks for the help! The problem was in d4d9007b293ccbecdf160de3e63a6b57a749cb60, and after a couple more modifications it builds locally.
CI is green. @KiterLuc can we merge it?