[smart_ptr/static_pointer_cast] Trying to make ADL useful in my boost libraries
Hi to all, There were some recent Shmem/Interprocess bug reports concerning header ordering issues. I've investigated it a bit and I've come up with a reduced test case that reproduces the problem that happens with the static_pointer_cast() expression. Now that I have reduced the problem, I don't know how to solve it so I need some expert ADL recommendation to implement a "best practices" approach in my boost code. Imagine a container that can store raw pointers or smart pointers pointer to a polymorphic class (or a container using an allocator that defines its own "pointer" type). To avoid instantiations, it uses static_pointer_cast to work only with base classes and avoid code bloat. This is typical in node based containers like lists and trees. Then suppose a container factory that creates containers based on a configuration. Here is the code: //////////////////////////////////////// //containers.hpp header: //////////////////////////////////////// namespace boost { namespace dummy { template<class Pointer> class container { Pointer ptr; public: void func() { static_pointer_cast<int>(Pointer()); } }; } //namespace dummy { } //namespace boost { //////////////////////////////////////// //smart_ptr.hpp header: //////////////////////////////////////// namespace boost { namespace dummy { template<class T> class smart_ptr { }; template<class T, class U> smart_ptr<T> static_pointer_cast(const smart_ptr<U> &u) { return smart_ptr<T>(); } } //namespace dummy { } //namespace boost { //////////////////////////////////////// //container_factory.hpp header: //////////////////////////////////////// namespace boost { namespace dummy { class container_factory { public: template<class Container> static Container *create() { return new Container; } }; } //namespace dummy { } //namespace boost { //////////////////////////////////////// //main.cpp: //////////////////////////////////////// #include "container.h" #include "smart_ptr.h" #include "container_factory.h" int main() { using namespace boost::dummy; typedef container<smart_ptr<int> > my_container; my_container *cont = container_factory::create<my_container>(); cont->func(); return 0; } This code compiles in VC 7.1 and VC 8 but fails with gcc, complaining the "there is no declaration for static_pointer_cast". If we change the header order to: #include "smart_ptr.h" #include "container.h" #include "container_factory.h" This compiles fine because the static_pointer_cast definition is found before the call. My first question is: * Is this gcc error correct? Revising a bit ADL + template issues (a fairly complicated logic that should be removed from C++, in my opinion) I think that gcc is right. * If gcc is correct, what is the implementor supposed to do? Every templatized class header shouldn't include smart_ptr.h because smart_ptr is unknown and it can even be a user-defined class. * If this is correct, that is the user supposed to do? The user has included all the headers, and he shouldn't know about the implementation and internal dependency issues. It's really a mess for a user to start guessing the correct header order. And this is only a simple dependency, imagine a fairly more complicated approach, it would be really a nightmare. So the question is, how should I write my library to take advantage of ADL without creating such header dependency problems? Should I avoid ADL and require static_pointer_cast overloads in boost namespace for a user class defined in a different namespace? I must have missed something, because otherwise, I would find ADL really useless as a robust customization point. Any expert willing to help? Regards, Ion
Ion Gaztañaga wrote: [...]
template<class Pointer> class container { Pointer ptr; public: void func() { static_pointer_cast<int>(Pointer()); } };
} //namespace dummy { } //namespace boost {
[...]
This code compiles in VC 7.1 and VC 8 but fails with gcc, complaining the "there is no declaration for static_pointer_cast". If we change the header order to:
#include "smart_ptr.h" #include "container.h" #include "container_factory.h"
This compiles fine because the static_pointer_cast definition is found before the call. My first question is:
* Is this gcc error correct? Revising a bit ADL + template issues (a fairly complicated logic that should be removed from C++, in my opinion) I think that gcc is right.
Questionable. The problem is that for static_pointer_cast<int> to be valid, static_pointer_cast must be known to be a template. It is understandable why g++ rejects it without a declaration, but I'm not sure whether this is conforming now, much less whether it'd be conforming tomorrow if there are open core issues for this case. Even if static_pointer_cast<int> is parsed correctly, I recall a core issue about whether the call should be subject to ADL.
* If gcc is correct, what is the implementor supposed to do? Every templatized class header shouldn't include smart_ptr.h because smart_ptr is unknown and it can even be a user-defined class.
To make the above work for raw pointers (these don't have an associated namespace), the implementor would need to #include boost/pointer_cast.hpp and add an using declaration for boost::static_pointer_cast. This may or may not make g++ happy.
I must have missed something, because otherwise, I would find ADL really useless as a robust customization point.
The typical ADL expression doesn't have an explicit <int> template parameter.
* Is this gcc error correct? Revising a bit ADL + template issues (a fairly complicated logic that should be removed from C++, in my opinion) I think that gcc is right.
Questionable. The problem is that for static_pointer_cast<int> to be valid, static_pointer_cast must be known to be a template. It is understandable why g++ rejects it without a declaration, but I'm not sure whether this is conforming now, much less whether it'd be conforming tomorrow if there are open core issues for this case.
I think I should post this to c.s.c++ to see if this call should kick ADL. If so, I will report a bug to the gcc mailing list. Anyway, static_pointer_cast is not a portable ADL function in Boost. What should we do? Declare an overload taking an additional dummy parameter to deduce the return type? template<class Source, class Target> smart_ptr<Target> static_pointer_cast (const smart_ptr<Source> &s, const Target &); so that the ADL call is: Pointer p; IntPtr ip = static_pointer_cast(p, int()); Meanwhile, I would need to predeclare static_pointer_cast in the Interprocess/Shmem headers for Interprocess/Shmem smart pointer types to make the code work in gcc, but static_pointer_cast is broken for ADL in Boost and that's bad news. Regards, Ion
Ion Gaztañaga wrote:
* Is this gcc error correct? Revising a bit ADL + template issues (a fairly complicated logic that should be removed from C++, in my opinion) I think that gcc is right.
Questionable. The problem is that for static_pointer_cast<int> to be valid, static_pointer_cast must be known to be a template. It is understandable why g++ rejects it without a declaration, but I'm not sure whether this is conforming now, much less whether it'd be conforming tomorrow if there are open core issues for this case.
I think I should post this to c.s.c++ to see if this call should kick ADL. If so, I will report a bug to the gcc mailing list. Anyway, static_pointer_cast is not a portable ADL function in Boost. What should we do? Declare an overload taking an additional dummy parameter to deduce the return type?
template<class Source, class Target> smart_ptr<Target> static_pointer_cast (const smart_ptr<Source> &s, const Target &);
so that the ADL call is:
Pointer p;
IntPtr ip = static_pointer_cast(p, int());
We used to write template<class Source, class Target> smart_ptr<Target> static_pointer_cast (const smart_ptr<Source> &s, Target * = 0); IntPtr ip = static_pointer_cast(p, (int*)0); when MSVC didn't support explicit template arguments. Another option is template<class Source, class Target> void static_pointer_cast ( const smart_ptr<Source> &s, smart_ptr<Target> & t ); Have you tried #include <boost/pointer_cast.hpp> // in class { using boost::static_pointer_cast; static_pointer_cast<T>( ps ); } under g++? Does it still fail? This is required to support the case where ps is a raw pointer.
Have you tried
#include <boost/pointer_cast.hpp>
// in class
{ using boost::static_pointer_cast; static_pointer_cast<T>( ps ); }
under g++? Does it still fail? This is required to support the case where ps is a raw pointer.
This also fails. Simplified test case: //////////////////////////////////////// //raw_ptr.hpp header: //////////////////////////////////////// namespace raw_ptr { //static_pointer_cast overload for raw pointers template<class T, class U> inline T* static_pointer_cast(U *ptr) { return static_cast<T*>(ptr); } } //namespace raw_ptr //////////////////////////////////////// //container.hpp header: //////////////////////////////////////// namespace dummy { template<class Pointer> class container { public: void func() { using raw_ptr::static_pointer_cast; static_pointer_cast<int>(Pointer()); } }; } //namespace dummy { //////////////////////////////////////// //smart_ptr.hpp header: //////////////////////////////////////// namespace dummy { template<class T> class smart_ptr {}; template<class T, class U> smart_ptr<T> static_pointer_cast(const smart_ptr<U> &u) { return smart_ptr<T>(); } } //namespace dummy { //////////////////////////////////////// //main.cpp //////////////////////////////////////// int main() { using namespace dummy; typedef container<smart_ptr<int> > my_container; my_container cont; cont.func(); typedef container<int *> my_container2; my_container2 cont2; cont2.func(); return 0; } This compiles fine with with VC 7.1. g++ fails with: "no matching function for call to 'static_pointer_cast(dummy::smart_ ptr<int>)'" Regards, Ion
Ion Gaztañaga wrote:
This compiles fine with with VC 7.1. g++ fails with: "no matching function for call to 'static_pointer_cast(dummy::smart_ ptr<int>)'"
This means that g++ doesn't do ADL for static_pointer_cast<int>. There's an issue about it by Gabriel Dos Reis: http://www.open-std.org/JTC1/SC22/WG21/docs/cwg_active.html#485 but it's still open and contains no further notes, so it isn't clear whether g++ is right or not.
This means that g++ doesn't do ADL for static_pointer_cast<int>. There's an issue about it by Gabriel Dos Reis:
http://www.open-std.org/JTC1/SC22/WG21/docs/cwg_active.html#485
but it's still open and contains no further notes, so it isn't clear whether g++ is right or not.
Too late... I've already posted in c.s.c++ ;-) Not being an expert, I think that gcc approach is quite limiting. If gcc's approach is considered correct, I think that we would need to invent a mechanism to make pointer casts compatible with ADL. Thanks for the help, Ion
On Jun 14, 2006, at 3:20 PM, Ion Gaztañaga wrote:
This means that g++ doesn't do ADL for static_pointer_cast<int>. There's an issue about it by Gabriel Dos Reis:
http://www.open-std.org/JTC1/SC22/WG21/docs/cwg_active.html#485
but it's still open and contains no further notes, so it isn't clear whether g++ is right or not.
Too late... I've already posted in c.s.c++ ;-) Not being an expert, I think that gcc approach is quite limiting. If gcc's approach is considered correct, I think that we would need to invent a mechanism to make pointer casts compatible with ADL.
Fwiw, my money is on gcc. It agrees with both CodeWarrior and EDG. But I'm anxiously awaiting confirmation on c.s.c++ too. :-) -Howard
Too late... I've already posted in c.s.c++ ;-) Not being an expert, I think that gcc approach is quite limiting. If gcc's approach is considered correct, I think that we would need to invent a mechanism to make pointer casts compatible with ADL.
Fwiw, my money is on gcc. It agrees with both CodeWarrior and EDG. But I'm anxiously awaiting confirmation on c.s.c++ too. :-)
If Codewarrior/EDG/gcc team wins, maybe the committee will be condemned to write new ADL friendly casting functions for shared_ptr<>, so be careful what you whish for... ;-) Regards, Ion
"Peter Dimov" <pdimov@mmltd.net> writes:
Ion Gaztañaga wrote:
This compiles fine with with VC 7.1. g++ fails with:
"no matching function for call to 'static_pointer_cast(dummy::smart_
ptr<int>)'"
This means that g++ doesn't do ADL for static_pointer_cast<int>.
Nor does any other compiler I know of, IIUC.
There's an
issue about it by Gabriel Dos Reis:
http://www.open-std.org/JTC1/SC22/WG21/docs/cwg_active.html#485
but it's still open and contains no further notes, so it isn't clear whether
g++ is right or not.
Well, if foo<bar>(...) starts doing ADL it's going to break a lot of code. I certainly leave off qualification when using that notation, knowing that ADL is avoided. -- Dave Abrahams Boost Consulting www.boost-consulting.com
Well, if foo<bar>(...) starts doing ADL it's going to break a lot of
code. I certainly leave off qualification when using that notation,
knowing that ADL is avoided.
Ok. I see that in c.s.c++ there is a similar response. Revising shared_ptr and intrusive_ptr code I see that all the functions except for cast functions are adequate for ADL (operators + get_pointer()). This is a pity because this makes some pointer-independent code hard in Boost. Interprocess/Shmem uses containers without supposing that allocator::pointer is a raw pointer and it uses static_pointer_cast to construct interprocess::list and interprocess:map classes avoiding template explosion. The current alternative would be: smart_ptr -> get_pointer (ADL) -> raw pointer -> static_cast -> raw_pointer -> smart_ptr(raw_pointer) Currently, I don't see any problem with this in Interprocess (obviously, this can be less efficient than having a specialized static cast function) but I think that we should have a new ADL-friendly cast mechanism in the smart_ptr library to write pointer independent code. These ADL-unfriendly cast functions make boost/pointer_cast.hpp useless in my opinion, as it was thought to be used for pointer independent code via ADL. And the alternatives like target = static_pointer_cast(source, (int*)0); look pretty bad comparing to the original target = static_pointer_cast<int>(source); Regards, Ion
Ion Gaztañaga <igaztanaga@gmail.com> writes:
it was thought to be used for pointer independent code via ADL
what is your basis for that assertion? -- Dave Abrahams Boost Consulting www.boost-consulting.com
what is your basis for that assertion?
Because I wrote that header thinking (wrongly, because of my ADL ignorance and VC support) that it would be used for ADL lookup: ////////////////////////////////////////////////////////////////////////////// // // (C) Copyright Ion Gaztañaga 2005. // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) // ////////////////////////////////////////////////////////////////////////////// #ifndef BOOST_POINTER_CAST_HPP #define BOOST_POINTER_CAST_HPP namespace boost { //static_pointer_cast overload for raw pointers template<class T, class U> inline T* static_pointer_cast(U *ptr) { return static_cast<T*>(ptr); } //.. } // namespace boost #endif //BOOST_POINTER_CAST_HPP I wrote this because I wanted to use it though ADL. If static_pointer_cast<Target>() can't be used through ADL, it's certainly a very limited header. Unless I know that my pointer can be a raw pointer, I know all the possible pointer types and I include all the headers, this header is useless. Until Boost 1.34 this header didn't exist. But I still see a need for customizable cast functions. Since shared_ptr is de-facto standard, I'm just proposing new generic cast functions. What about template specialization?: //pointer_cast.hpp namespace boost { template<class TargetPtr> class cast_to; template<class T> class cast_to<T*> { template<class Source> static T * using_static_cast(Source *source) { return static_cast<T*>(source); } } } //namespace boost { namespace boost { //smart_ptr.hpp template<class T> class cast_to<smart_ptr<T> > { template<class Source> static smart_ptr<T> using_static_cast(const smart_ptr<Source> &source); } } //namespace boost { // pointer independent code BasePtr d_ptr = ... typedef typename boost::pointer_to_other<BasePtr, Base>::type DerivedPtr; DerivedPtr target = cast_to<DerivedPtr>::using_static_cast(b_ptr); //////////////////////////////// Sorry if there are errors in the code. Any suggestion? Time to request cast operator overloading for user classes? Regards, Ion
Ion Gaztañaga wrote:
what is your basis for that assertion?
Because I wrote that header thinking (wrongly, because of my ADL ignorance and VC support) that it would be used for ADL lookup:
////////////////////////////////////////////////////////////////////////////// // // (C) Copyright Ion Gaztañaga 2005. // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) // //////////////////////////////////////////////////////////////////////////////
#ifndef BOOST_POINTER_CAST_HPP #define BOOST_POINTER_CAST_HPP
namespace boost {
//static_pointer_cast overload for raw pointers template<class T, class U> inline T* static_pointer_cast(U *ptr) { return static_cast<T*>(ptr); }
//..
} // namespace boost
#endif //BOOST_POINTER_CAST_HPP
I wrote this because I wanted to use it though ADL. If static_pointer_cast<Target>() can't be used through ADL, it's certainly a very limited header. Unless I know that my pointer can be a raw pointer, I know all the possible pointer types and I include all the headers, this header is useless. Until Boost 1.34 this header didn't exist.
But I still see a need for customizable cast functions. Since shared_ptr is de-facto standard, I'm just proposing new generic cast functions. What about template specialization?:
//pointer_cast.hpp
namespace boost {
template<class TargetPtr> class cast_to;
template<class T> class cast_to<T*> { template<class Source> static T * using_static_cast(Source *source) { return static_cast<T*>(source); } }
} //namespace boost {
namespace boost {
//smart_ptr.hpp
template<class T> class cast_to<smart_ptr<T> > { template<class Source> static smart_ptr<T> using_static_cast(const smart_ptr<Source> &source); }
} //namespace boost {
// pointer independent code
BasePtr d_ptr = ...
typedef typename boost::pointer_to_other<BasePtr, Base>::type DerivedPtr;
DerivedPtr target = cast_to<DerivedPtr>::using_static_cast(b_ptr);
////////////////////////////////
Sorry if there are errors in the code. Any suggestion? Time to request cast operator overloading for user classes?
May it be simplified to the following? template<class T> struct type {}; template<class T, class U> inline T* static_pointer_cast_impl(U *ptr, type<T>) { return static_cast<T*>(ptr); } //static_pointer_cast overload for raw pointers template<class T, class U> inline T* static_pointer_cast(U *ptr) { return static_pointer_cast_impl(ptr, type<T>()); // ADL kicks in here }
Maxim Yegorushkin wrote:
Ion Gaztañaga wrote:
what is your basis for that assertion? Because I wrote that header thinking (wrongly, because of my ADL ignorance and VC support) that it would be used for ADL lookup:
////////////////////////////////////////////////////////////////////////////// // // (C) Copyright Ion Gaztañaga 2005. // Distributed under the Boost Software License, Version 1.0. // (See accompanying file LICENSE_1_0.txt or copy at // http://www.boost.org/LICENSE_1_0.txt) // //////////////////////////////////////////////////////////////////////////////
#ifndef BOOST_POINTER_CAST_HPP #define BOOST_POINTER_CAST_HPP
namespace boost {
//static_pointer_cast overload for raw pointers template<class T, class U> inline T* static_pointer_cast(U *ptr) { return static_cast<T*>(ptr); }
//..
} // namespace boost
#endif //BOOST_POINTER_CAST_HPP
I wrote this because I wanted to use it though ADL. If static_pointer_cast<Target>() can't be used through ADL, it's certainly a very limited header. Unless I know that my pointer can be a raw pointer, I know all the possible pointer types and I include all the headers, this header is useless. Until Boost 1.34 this header didn't exist.
But I still see a need for customizable cast functions. Since shared_ptr is de-facto standard, I'm just proposing new generic cast functions. What about template specialization?:
//pointer_cast.hpp
namespace boost {
template<class TargetPtr> class cast_to;
template<class T> class cast_to<T*> { template<class Source> static T * using_static_cast(Source *source) { return static_cast<T*>(source); } }
} //namespace boost {
namespace boost {
//smart_ptr.hpp
template<class T> class cast_to<smart_ptr<T> > { template<class Source> static smart_ptr<T> using_static_cast(const smart_ptr<Source> &source); }
} //namespace boost {
// pointer independent code
BasePtr d_ptr = ...
typedef typename boost::pointer_to_other<BasePtr, Base>::type DerivedPtr;
DerivedPtr target = cast_to<DerivedPtr>::using_static_cast(b_ptr);
////////////////////////////////
Sorry if there are errors in the code. Any suggestion? Time to request cast operator overloading for user classes?
May it be simplified to the following?
[] Please ignore the previous code. Kindly consider the following: template<class T> struct type {}; // static_pointer_cast_impl overload for raw pointers template<class T, class U> inline T* static_pointer_cast_impl(U *ptr, type<T>) { return static_cast<T*>(ptr); } // static_pointer_cast ADL dispatcher function template<class T, class U> inline T static_pointer_cast(U u) { return static_pointer_cast_impl(u, type<T>()); // ADL kicks in here } // ... template<class T> class smart_ptr { /* ... */ }; // static_pointer_cast_impl overload for raw smart_ptr template<class T, class U> inline smart_ptr<T> static_pointer_cast_impl(smart_ptr<U> const& p, type<T>) { return smart_ptr<T>(p.get()); }
Please ignore the previous code. Kindly consider the following:
template<class T> struct type {};
// static_pointer_cast_impl overload for raw pointers template<class T, class U> inline T* static_pointer_cast_impl(U *ptr, type<T>) { return static_cast<T*>(ptr); }
// static_pointer_cast ADL dispatcher function template<class T, class U> inline T static_pointer_cast(U u) { return static_pointer_cast_impl(u, type<T>()); // ADL kicks in here }
// ...
template<class T> class smart_ptr { /* ... */ };
// static_pointer_cast_impl overload for raw smart_ptr template<class T, class U> inline smart_ptr<T> static_pointer_cast_impl(smart_ptr<U> const& p, type<T>) { return smart_ptr<T>(p.get()); }
Seems ok, although "type<T>" could be something like "cast_to_pointer<T>". Whatever we choose, I think we should add it to "shared_ptr" and "intrusive_ptr", and define well-known boost ADL casting functions. Current "xxx_pointer_cast" is in TR1, so I don't know if we have arrived late for an standard C++ ADL friendly cast functions. Peter? Regards, Ion
Maxim Yegorushkin <maxim.yegorushkin@gmail.com> writes: <snip entire foregoing post>
Sorry if there are errors in the code. Any suggestion? Time to request cast operator overloading for user classes?
May it be simplified to the following?
[]
Please ignore the previous code. Kindly consider the following:
... http://www.boost.org/more/discussion_policy.htm#effective Thank you. -- Dave Abrahams Boost Consulting www.boost-consulting.com
participants (5)
-
David Abrahams -
Howard Hinnant -
Ion Gaztañaga -
Maxim Yegorushkin -
Peter Dimov