Avoid compilation error when passing in heap_t to C++ allocators

[rHermes]

Before it would not work to create the mi_heap_stl_allocator types with passing in a mi_heap_t*, since sizeof is used and it gives a compilation error. This change fixes that.

Steps to reproduce the current error:

include <mimalloc.h>
#include <vector>

int main() {
    mi_heap_t *hp = mi_heap_new();
    mi_heap_destroy_stl_allocator<int> thisOne(hp);
    mi_heap_destroy(hp);
}

Compilation error is:

====================[ Build | mimalloc-stuff | Debug ]==========================
/home/rhermes/.local/share/JetBrains/Toolbox/apps/clion/bin/cmake/linux/x64/bin/cmake --build /home/rhermes/commons/projects/measure-everything/cmake-build-debug --target mimalloc-stuff -j 22
[1/2] Building CXX object CMakeFiles/mimalloc-stuff.dir/mimalloc-stuff/main.cpp.o
FAILED: CMakeFiles/mimalloc-stuff.dir/mimalloc-stuff/main.cpp.o 
/usr/bin/c++  -I/home/rhermes/commons/projects/measure-everything/cmake-build-debug/_deps/mimalloc-src/include -g -std=gnu++20 -fdiagnostics-color=always -MD -MT CMakeFiles/mimalloc-stuff.dir/mimalloc-stuff/main.cpp.o -MF CMakeFiles/mimalloc-stuff.dir/mimalloc-stuff/main.cpp.o.d -o CMakeFiles/mimalloc-stuff.dir/mimalloc-stuff/main.cpp.o -c /home/rhermes/commons/projects/measure-everything/mimalloc-stuff/main.cpp
In file included from /usr/include/c++/13.2.1/bits/shared_ptr.h:53,
                 from /usr/include/c++/13.2.1/memory:80,
                 from /home/rhermes/commons/projects/measure-everything/cmake-build-debug/_deps/mimalloc-src/include/mimalloc.h:489,
                 from /home/rhermes/commons/projects/measure-everything/mimalloc-stuff/main.cpp:5:
/usr/include/c++/13.2.1/bits/shared_ptr_base.h: In instantiation of ‘std::__shared_ptr<_Tp, _Lp>::__shared_ptr(_Yp*) [with _Yp = mi_heap_s; <template-parameter-2-2> = void; _Tp = mi_heap_s; __gnu_cxx::_Lock_policy _Lp = __gnu_cxx::_S_atomic]’:
/usr/include/c++/13.2.1/bits/shared_ptr.h:214:46:   required from ‘std::shared_ptr<_Tp>::shared_ptr(_Yp*) [with _Yp = mi_heap_s; <template-parameter-2-2> = void; _Tp = mi_heap_s]’
/home/rhermes/commons/projects/measure-everything/cmake-build-debug/_deps/mimalloc-src/include/mimalloc.h:497:50:   required from ‘_mi_heap_stl_allocator_common<T, _mi_destroy>::_mi_heap_stl_allocator_common(mi_heap_t*) [with T = int; bool _mi_destroy = true; mi_heap_t = mi_heap_s]’
/home/rhermes/commons/projects/measure-everything/cmake-build-debug/_deps/mimalloc-src/include/mimalloc.h:550:91:   required from ‘mi_heap_destroy_stl_allocator<T>::mi_heap_destroy_stl_allocator(mi_heap_t*) [with T = int; mi_heap_t = mi_heap_s]’
/home/rhermes/commons/projects/measure-everything/mimalloc-stuff/main.cpp:10:50:   required from here
/usr/include/c++/13.2.1/bits/shared_ptr_base.h:1472:26: error: invalid application of ‘sizeof’ to incomplete type ‘mi_heap_s’
 1472 |           static_assert( sizeof(_Yp) > 0, "incomplete type" );
      |                          ^~~~~~~~~~~
ninja: build stopped: subcommand failed.

[rHermes]

I'm testing more now, and this doesn't really work at all, even with the given fix.

The current approach of using a std::shared_ptr to manage the heap only works if we are going to call a destructor on it. We also have to change the deleter when constructing. I'll do that and add tests for this and push again.

[rHermes]

@microsoft-github-policy-service agree

[rHermes]

@daanx requesting a PR here, if you have the time :)

[daanx]

Hi @rHermes -- just to confirm, this is only a problem for mi_heap_destroy_stl_allocator right? (On msvc 2022 I only get errors on the "destroy" variant).

This would be great to fix.. but we cannot pull in mimalloc/types.h as that breaks the abstraction barrier where mi_heap_t should be abstract. I guess you pulled it in to make the sizeof operation in the static assertion pass? Is there another way we could make this work where we do not include the internal mimalloc/types.h header file?

Maybe we should not use std::shared_ptr ? Or perhaps use a wrapper struct around a mi_heap_t* ?

[rHermes]

Hey!

Yes the problem is only for mi_heap_destroy_stl_allocator and it's only when you pass in a heap type. The problem here is that shared_ptr is a perfect type for this, as the allocator might live on in objects created with it, like std::vector or others. I think the approach works well and I don't think it should be changed.

I don't really know about the internals of mimalloc, so I don't know about why it needs to be upheld, but I will try some ways to do it, without including the type! I'll comment back when I either push a new version or give up ;)

[rHermes]

Ok, that was quick. I don't know why I included the type initially, but it's not needed. I've removed it now and it's good!

[daanx]

ah that's great. Testing now

[daanx]

Thanks so much!