id,summary,reporter,owner,description,type,status,milestone,component,version,severity,resolution,keywords,cc
9948,remove use of const_cast in intrusive containers,Matei David <matei@…>,Ion Gaztañaga,"In intrusive containers, the existing value traits customization mechanism described  [http://www.boost.org/doc/libs/1_55_0/doc/html/intrusive/value_traits.html here] allows for value `pointer` to be a custom class. Moreover, the existing pointer traits mechanism allows for the user to:

- Define and use a custom `pointer_traits<pointer>::reference` class instead of raw references.
- Provide custom conversions between `pointer` and `reference`, and between `pointer` and `const_pointer`.

That being said, in `list.hpp`, `slist.hpp`, `bstree.hpp` the C++ operator `const_cast` operator is used to convert a `const_reference` (defined as `pointer_traits<const_pointer>::reference`) into a `reference` (defined as `pointer_traits<pointer>::reference`). This happens in `[s_]iterator_to()` methods. This is unfortunate because `const_cast` cannot be overridden, and it cannot work with anything other than real pointers and real references. So even though most of the existing code base (through value traits & pointer traits mechanisms) can support custom references, that possibility is removed by the use of `const_cast`.

I created a pull request in that sense, where I replaced calls of the form:

`const_cast<reference>(value)` (where `value` is a `pointer_traits<const_pointer>::reference`)

by:

`*pointer_traits<pointer>::const_cast_from(pointer_traits<const_pointer>::pointer_to(value))`

This relies on the existence of:

1. `const_pointer pointer_traits<const_pointer>::pointer_to(const_reference)`;
2. `pointer pointer_traits<pointer>::const_cast_from(const_pointer)`;
3. `reference pointer::operator * ()`.

I think it is reasonable to expect all of these to be met. A user of raw references and raw pointers will be unaffected, because the default definitions of `pointer_traits` work as expected. As explained earlier, there is no existing code that uses custom references. That leaves only code with custom pointers and raw references to worry about breaking.

Regarding 1, the `pointer_to()` method is used heavily by the boost intrusive code, so it is reasonable to expect any existing specialization of `pointer_traits` to provide that.

Regarding 2, the `const_cast_from()` method, even though clearly documented in the pointer traits description, is not otherwise used in boost intrusive. So it is possible for existing code to specialize `pointer_traits` and not provide `const_cast_from`, which would result in broken code with the current modifications. I think the error message would be quite informative though: ""missing const_cast_from"", and easy to diagnose and fix. It would also be a step in the right direction: if a user really needs custom pointer traits, then they should really be providing the custom casts.

As for 3, the `operator * ()` method, I think it's reasonable to expect a custom pointer class would provide that. Standard smart pointer classes provide that. In the future, I would also suggest adding this conversion explicitly in `pointer_traits`, e.g., require specializations to define `pointer_traits<pointer>::reference pointer_traits<pointer>::reference_from(pointer)`.

The pull request is [https://github.com/boostorg/intrusive/pull/4 here]. As explained, this removes `const_cast` from `list.hpp`, `slist.hpp`, and `bstree.hpp`. It is still used in `hashtable.hpp`, whose code looks a bit different than the other containers, so I left it alone.
",Feature Requests,closed,To Be Determined,intrusive,Boost 1.55.0,Problem,fixed,intrusive custom references,