Revert "Use sizeof(char) in place for sizeof(void) (#515)"

[hidmic]

This reverts commit c3769dd.

I was wrong. void allocators shouldn't behave like char allocators. See ros2/rclcpp#1657.

[hidmic]

@ros-pull-request-builder retest this please

[hidmic]

CI up to demo_nodes_cpp:

• Linux
• Linux-aarch64
• macOS
• Windows

[clalancette]

I may not be qualified to review this, but I'm trying to reconcile a couple of different statements made around this area:

• Do not attempt to use void allocators for memory allocation. rclcpp#1657 (comment) says "it doesn't cover custom void allocators (which shouldn't have allocate nor deallocate methods to begin with)."
• https://en.cppreference.com/w/cpp/memory/allocator says "The explicit specialization for void lacks the member typedefs reference, const_reference, size_type and difference_type. This specialization declares no member functions. " (which agrees with the above statement)
• This file (allocator_tutorial.cpp) declares struct MyAllocator, which by default has typename T = void, and also has implementations of allocate and deallocate. Doesn't that directly contradict the above two statements?

[hidmic]

This file (allocator_tutorial.cpp) declares struct MyAllocator, which by default has typename T = void, and also has implementations of allocate and deallocate. Doesn't that directly contradict the above two statements?

Exactly. It shouldn't have them. Alas, many Allocator implementations (the one in allocator_tutorial.cpp, the one in rclcpp tests, tlsf_cpp's, libcxx's) do not go through the hassle of partial specialization when value_type is void. Instead, they roll with it knowing that (a) no downstream code should attempt to allocate or deallocate using a void allocator, and (b) sizeof(void) is ill-formed anyways and it won't compile (unless you're using gcc with GNU extensions, which do allow it). You can still use such void allocators so long as allocate never gets addressed (because a C++ compiler will not compile its definition if it knows it's not being used i.e. it will partially instantiate the template).

[clalancette]

OK, thanks, that makes some sense. Just be totally sure I understand, then, the problem here is not really that MyAllocator has an allocate and deallocate method, its that it should never be allowed to be instantiated with a void type. We pretty much know that doesn't happen, otherwise it wouldn't compile on clang/MSVC. So is the right thing to do here to totally remove the default template type as void? Wouldn't that still work, but get rid of the false impression that this works for void?

[hidmic]

Just be totally sure I understand, then, the problem here is not really that MyAllocator has an allocate and deallocate method, its that it should never be allowed to be instantiated with a void type. We pretty much know that doesn't happen, otherwise it wouldn't compile on clang/MSVC.

It is being instantiated with a void type, but only to pass around type erased allocators (which user code rebinds), not to allocate memory.

[hidmic]

Alright, thanks for the reviews ! Going in.