On 26/09/18 19:31 +0200, Florian Weimer wrote:
* Jonathan Wakely:
Does anybody see any problems with this change?
Can you put a debug assert into _M_add_reference and
_M_remove_reference, to check that they are never called on an
_S_classic object? Or put the check directly into those functions?
I thought about putting it directly in those functions, as that would
ensure we don't get it wrong. But those functions are defined inline
in the header, so there is already user code calling them that has
inlined the functions. That existing code won't contain the checks,
and so would continue to update reference counts on the _S_classic
object, potentially deleting it while it's still in use.
We could put debug assertions in there, but I prefer not to pessimise
user code with checks that our implementation is correct. Maybe in
this case the benefits outweight the cost. This code rarely changes
though, so I am not very concerned that somebody will add new updates
of the reference counts and forget to check for == _S_classic.
I did review all uses of locale::_Impl:_M_(add|remove)_reference, and
the only one that I didn't change (and the only one which is in a
header and so can't be altered in existing user code) is in the
locale(const locale& __other, _Facet* __f) constructor in
<bits/locale_classes.tcc>. We know we've just created a new _Impl with
a reference count of 1, so it's not the _S_classic object, and
decrementing the reference count unconditionally is correct.
