<xnode_handle.h>: values should be destroyed with value_type allocator

[Kamichanw]

for example, in the function _Clear

STL/stl/inc/xnode_handle.h

Lines 82 to 91 in 5f3e912

	void _Clear() noexcept { // destroy any contained node and return to the empty state
	if (_Ptr) {
	_Alloc& _Al = _Getal();
	_Alnode _Node_alloc{_Al};
	_Alnode_traits::destroy(_Node_alloc, _STD addressof(_Ptr->_Myval));
	_Node::_Freenode0(_Node_alloc, _Ptr);
	_Destroy_in_place(_Al);
	_Ptr = nullptr;
	}
	}

why uses node allocator traits(_Alnode_traits) to destroy _Ptr->_Myval? I think it should be made by value allocator traits(_Alty_traits). All of value destructions have this problem.

Also tracked by DevCom-1190997 and Microsoft-internal VSO-1217757 / AB#1217757 .

[CaseyCarter]

The Standard is pretty clear that ator_traits (an alias for allocator_traits<allocator_type>, where allocator_type comes from the container) is used to destroy the value stored in the node in both places where that occurs ([container.node.cons]/3.1 and [container.node.dtor]/1).

[BillyONeal]

I think the standard is wrong here: the allocator requirements don't require that the T* passed to construct or destroy match allocator::value_type.

If we want to ensure that it matches allocator::value_type, then we and the standard should consistently do that everywhere (and the allocator requirements should contain that restriction). But given that nothing has cared in the last 30 years I think the performance cost of the extra rebinds, small as it may be, is not worthwhile and the standard should resolve this by not mentioning the allocator_traits type used.

[CaseyCarter]

I agree, but I don't care enough to invest my time on an LWG issue for this. We're already paying the rebind cost since the Standard requires node-handles to store a value_type allocator, so the change to conform here is trivial.

If you feel strongly, feel free to file an LWG issue.

[BillyONeal]

Casey points out that other parts of the standard have been updated to be consistent about this in his PR, so I don't think an LWG issue is needed.

[Kamichanw]

The Standard is pretty clear that ator_traits (an alias for allocator_traits<allocator_type>, where allocator_type comes from the container) is used to destroy the value stored in the node in both places where that occurs ([container.node.cons]/3.1 and [container.node.dtor]/1).

Yes, but in the <xnode_handle.h>, _Alnode_traits is traits of node allocator (using _Alnode = _Rebind_alloc_t<_Alloc, _Node>), which isn't conform with standard.

[BillyONeal]

Yes, but in the <xnode_handle.h>, _Alnode_traits is traits of node allocator (using _Alnode = _Rebind_alloc_t<_Alloc, _Node>), which isn't conform with standard.

I'm arguing that the implementation is correct and the standard is wrong, because the containers don't do that rebind, so it's strange for node_handle to do the extra rebind but the containers to not do so.

[CaseyCarter]

The Standard is pretty clear that ator_traits (an alias for allocator_traits<allocator_type>, where allocator_type comes from the container) is used to destroy the value stored in the node in both places where that occurs ([container.node.cons]/3.1 and [container.node.dtor]/1).

Yes, but in the <xnode_handle.h>, _Alnode_traits is traits of node allocator (using _Alnode = _Rebind_alloc_t<_Alloc, _Node>), which isn't conform with standard.

Sorry for the lack of clarity: I was pointing out the justification in the Standard for your "I think this is a bug" statement. Billy is bothered by the fact that the Standard is telling us to do something that's semantically inconsistent with how nodes are handled by containers - it's weird that we have to handle nodes in a container differently from nodes outside a container held in a node-handle.

It's almost a tradition of WG21 that every time the Standard Library uses allocators it uses them a bit differently than in the other places. I don't think this indicates disagreement so much as a lack of clarity about the allocator design.

[BillyONeal]

I don't think this indicates disagreement so much as a lack of clarity about the allocator design.

Or lack thereof 😅