Opened 8 years ago

Closed 8 years ago

#10853 closed Bugs (fixed)

problem with detection of const_cast_from

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

Description

According to the documentation for the boost::intrusive::pointer_traits non-standard extensions, pointer_traits<Ptr>::const_cast_from(const UPtr&) should try to call Ptr::const_cast_from(const UPtr&) if it exists, or else use a default. It seems to me that this process fails with compilation error (not only misdetection) when const_cast_from() is a plain static function inside Ptr, as opposed to a function template. To trigger this process and the compilation error, use e.g. front() on an intrusive list.

For an example demonstrating this, look at test/bounded_pointer.hpp in the boost intrusive git repo. There, const_cast_from is a template:

template<class U> static bounded_pointer const_cast_from(const bounded_pointer<U> &)

For me (gcc-4.9.2, clang-3.4), the compilation fails if the signature is changed to:

static bounded_pointer const_cast_from(const bounded_pointer<const_val_t> &)

For a real program showing this, use the non-dummy version of test/test_list.cpp. (I don't understand why it was commented out.) To clarify, the problem has nothing to do with bounded_pointer, that's just an existing file demonstrating it.

Digging a bit into this issue, it appears that the problem is with the mechanism used to detect the presence const_cast_from() inside Ptr. That mechanism is in boost/intrusive/detail/has_member_function_callable_with.hpp. In my case, the error comes from line 200 of that file, saying that the call to const_cast_from is ambiguous: one candidate is the real function, the other candidate is the dummy declared on line 191. I believe the whole point is for overload resolution to prefer the real function to the dummy, which doesn't happen in this case.

Attachments (1)

const-cast-from-failure.cpp (277 bytes ) - added by Matei David <matei@…> 8 years ago.

Download all attachments as: .zip

Change History (5)

by Matei David <matei@…>, 8 years ago

Attachment: const-cast-from-failure.cpp added

comment:1 by Matei David <matei@…>, 8 years ago

I've isolated the heart of the problem. The attached file demonstrates how overload resolution fails with 2 non-template candidate functions, when both involve non-trivial constructor calls, even though one of them involves ellipses. I imagine the original detection code in boost/intrusive/detail/has_member_function_callable_with.hpp assumed the compiler would prefer the non-ellipses constructor.

The bug is triggered in the boost intrusive library as follows:

  1. In the docs, the signature for list_node_traits::get_next() is:
    static node_ptr get_next(const_node_ptr)
    
  2. In list::front() const, the following code appears:
    detail::uncast(node_traits::get_next(this->get_root_node()))
    
  3. As a result, detail::uncast() is called with an argument of type node_ptr (not const_node_ptr!).
  4. In turn, this triggers the mechanism looking for:
    node_ptr::const_cast_from(const node_ptr&)
    
  5. The documentation doesn't specify such a method should exist. One would assume it is ok to only have a similar function where the argument is const_node_ptr (not node_ptr!):
    node_ptr::const_cast_from(const const_node_ptr&)
    
  6. The existing code tries to detect const_cast_from by passing it an argument of type declval<node_ptr>().
  7. Naturally, a constructor exists with the signature:
    const_node_ptr(const node_ptr&)
    
  8. However, the overload resolution code now has to choose between calling the constructor above, or the ellipses constructor from has_member_function_callable_with.hpp:191.
  9. As the small example demonstrates, the resolution fails with a compile-time error.

Possible fixes:

  • Update the docs to make it clear node_ptr::const_cast_from(const node_ptr&) should exist. It can be either a normal function, or a template specialization. But crucially, it may not involve extra constructor calls that might confuse overload resolution.
  • Fix the intrusive code to not run unnecessary const_cast_from detection, i.e., when the argument is already a pointer to a non-const object. This is hard, because it's unclear where to look.
  • More robust const_cast_from detection.

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

const_cast_from must be a template because you can't only const_cast from a pointer to a const object but also from a non-const volatile object, const_cast also works for expressions that only remove volatile, etc.

I agree that list::front() does not need to use const_cast_from but this expression:

int *p = 0; const_cast<int*>(p);

is valid, and const_cast_from tried to emulate all this permitted behaviour (specially to support some generic code where pointers passed as argumnets could be non-const-qualified.

comment:3 by Matei David <matei@…>, 8 years ago

The non-template conversion function I was using (in 5 above) could do Ptr<A> to Ptr<A> conversion by using an intermediate Ptr<const A> object. The problem was not with the conversion function not being general enough, but with the pointer_traits code failing while detecting the presence of const_cast_from, because it involved an extra constructor call. This extra constructor call confused the overload resolution.

I don't mind const_cast_from being a template, what I minded was finding it out the hard way. Perhaps this could be better explained here: http://www.boost.org/doc/libs/1_57_0/doc/html/boost/intrusive/pointer_traits.html

Here's a suggestion:

When defining a custom family of pointers or references to be used with BI library, make sure the public static conversion functions accessed through the pointer_traits interface (*_cast_from and pointer_to) can properly convert between const and nonconst referred member types without the use of implicit constructor calls. It is suggested these conversions be implemented as function templates, where the template argument is the type of the object being converted from.

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

Resolution: fixed
Status: newclosed

I added your proposal to the pointer_traits documentation:

SHA-1: 50f9f57f6cc3c6d4f8cfe1c5a2967e297ad487db

  • Documented pointer_traits according to Trac #10853 proposal

Thanks.

Note: See TracTickets for help on using tickets.