Opened 8 years ago

Closed 8 years ago

#9948 closed Feature Requests (fixed)

remove use of const_cast in intrusive containers

Reported by: Matei David <matei@…> Owned by: Ion Gaztañaga
Milestone: To Be Determined Component: intrusive
Version: Boost 1.55.0 Severity: Problem
Keywords: intrusive custom references Cc:

Description

In intrusive containers, the existing value traits customization mechanism described 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 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.

Change History (1)

comment:1 by Ion Gaztañaga, 8 years ago

Resolution: fixed
Status: newclosed

Thanks. I've reviewed the patch and spotted several more const_cast removals (in unordered containers). Detected [s_]iterator_to was not tested in many containers and fixed it.

Fixed in develop branch: SHA-1: 7fc779ff0d28f0fcc603805eec00ac33442a045b

  • Fixed #9948: remove use of const_cast in intrusive containers.

Added tests for [s_]iterator_to.

Note: See TracTickets for help on using tickets.