Apparent STL implementation inconsistency

[David Bryant <daveb at acres dot com dot au>]

In class pointer_to_unary_function there is a direct correspondence
between the argument type of operator() and
unary_function::argument_type in its superclass.

i.e.,

    template <class _Arg, class _Result>
    struct unary_function {
	typedef _Arg argument_type;
	typedef _Result result_type;
    };


    template <class _Arg, class _Result>
    class pointer_to_unary_function : public unary_function<_Arg, _Result> {
    protected:
	_Result (*_M_ptr)(_Arg);
    public:
	pointer_to_unary_function() {}
	explicit pointer_to_unary_function(_Result (*__x)(_Arg)) :
	    _M_ptr(__x) {}
	_Result operator()(_Arg __x) const { return _M_ptr(__x); }
    };


    template <class _Arg, class _Result>
    inline pointer_to_unary_function<_Arg, _Result>
    ptr_fun(_Result (*__x)(_Arg)) {
	return pointer_to_unary_function<_Arg, _Result>(__x);
    }


    void some_function(int & i);
    int k = 5;
    ptr_fun(&some_function)(k);		// compiles fine
    
However, in mem_fun_ref_t, the correspondence between the argument type
of operator() and the unary_function::argument_type in its superclass
is wrong. It is a non-const reference in operator(), but
unary_function::argument_type is the value type.


    class mem_fun_ref_t : public unary_function<_Tp,_Ret> {
    public:
	explicit mem_fun_ref_t(_Ret (_Tp::*__pf)()) : _M_f(__pf) {}
	    _Ret operator()(_Tp& __r) const { return (__r.*_M_f)(); }
    private:
	    _Ret (_Tp::*_M_f)();
    };

I believe that this line:

    class mem_fun_ref_t : public unary_function<_Tp,_Ret> {

Should be replaced with

    class mem_fun_ref_t : public unary_function<_Tp &,_Ret> {

And likewise for all the others (const_mem_fun_t mem_fun1_ref_t, etc...)




Furthermore, binder1st shouldn't assume that the argument is a
reference:


    template <class _Operation>
    class binder1st : public unary_function<typename _Operation::second_argument_type,
					    typename _Operation::result_type> {
    protected:
	_Operation op;
	typename _Operation::first_argument_type value;
    public:
	binder1st(const _Operation& __x, const typename
		  _Operation::first_argument_type& __y) : op(__x), value(__y) { }
	typename _Operation::result_type
	    operator()(const typename _Operation::second_argument_type& __x) const {
	    return op(value, __x);
	}
    };

    template <class _Operation, class _Tp>
    inline binder1st<_Operation>
    bind1st(const _Operation& __oper, const _Tp& __x) {
	typedef typename _Operation::first_argument_type _Arg1_type;
	return binder1st<_Operation>(__oper, _Arg1_type(__x));
    }


because it breaks the following code


    void blah1(int &);
    void blah2(int &, int);

    void
    test() {
	int i, j;
	ptr_fun(&blah1)(j);			// compiles fine
	bind2nd(ptr_fun(&blah2), i)(j);		// doesn't compile
    }


I believe that the line: 

    operator()(const typename _Operation::second_argument_type& __x) const {

Should be replaced with:

    operator()(typename _Operation::second_argument_type __x) const {

And likewise for bind2nd

I believe that these changes would make gcc's STL implementation more
consistent. I have attached a patch to precisely summarise my contention.
What do other people think ?

David Bryant.
--- stl_function.h.bak	Thu Aug 30 09:20:23 2001
+++ stl_function.h	Thu Aug 30 09:22:55 2001
@@ -195,7 +195,7 @@
             const typename _Operation::first_argument_type& __y)
       : op(__x), value(__y) {}
   typename _Operation::result_type
-  operator()(const typename _Operation::second_argument_type& __x) const {
+  operator()(typename _Operation::second_argument_type __x) const {
     return op(value, __x); 
   }
 };
@@ -220,7 +220,7 @@
             const typename _Operation::second_argument_type& __y) 
       : op(__x), value(__y) {}
   typename _Operation::result_type
-  operator()(const typename _Operation::first_argument_type& __x) const {
+  operator()(typename _Operation::first_argument_type __x) const {
     return op(__x, value); 
   }
 };
@@ -503,7 +503,7 @@
 
 
 template <class _Ret, class _Tp>
-class mem_fun_ref_t : public unary_function<_Tp,_Ret> {
+class mem_fun_ref_t : public unary_function<_Tp &,_Ret> {
 public:
   explicit mem_fun_ref_t(_Ret (_Tp::*__pf)()) : _M_f(__pf) {}
   _Ret operator()(_Tp& __r) const { return (__r.*_M_f)(); }
@@ -512,7 +512,7 @@
 };
 
 template <class _Ret, class _Tp>
-class const_mem_fun_ref_t : public unary_function<_Tp,_Ret> {
+class const_mem_fun_ref_t : public unary_function<const _Tp &,_Ret> {
 public:
   explicit const_mem_fun_ref_t(_Ret (_Tp::*__pf)() const) : _M_f(__pf) {}
   _Ret operator()(const _Tp& __r) const { return (__r.*_M_f)(); }
@@ -540,7 +540,7 @@
 };
 
 template <class _Ret, class _Tp, class _Arg>
-class mem_fun1_ref_t : public binary_function<_Tp,_Arg,_Ret> {
+class mem_fun1_ref_t : public binary_function<_Tp &,_Arg,_Ret> {
 public:
   explicit mem_fun1_ref_t(_Ret (_Tp::*__pf)(_Arg)) : _M_f(__pf) {}
   _Ret operator()(_Tp& __r, _Arg __x) const { return (__r.*_M_f)(__x); }
@@ -549,7 +549,7 @@
 };
 
 template <class _Ret, class _Tp, class _Arg>
-class const_mem_fun1_ref_t : public binary_function<_Tp,_Arg,_Ret> {
+class const_mem_fun1_ref_t : public binary_function<const _Tp &,_Arg,_Ret> {
 public:
   explicit const_mem_fun1_ref_t(_Ret (_Tp::*__pf)(_Arg) const) : _M_f(__pf) {}
   _Ret operator()(const _Tp& __r, _Arg __x) const { return (__r.*_M_f)(__x); }
@@ -578,7 +578,7 @@
 };
 
 template <class _Tp>
-class mem_fun_ref_t<void, _Tp> : public unary_function<_Tp,void> {
+class mem_fun_ref_t<void, _Tp> : public unary_function<_Tp &,void> {
 public:
   explicit mem_fun_ref_t(void (_Tp::*__pf)()) : _M_f(__pf) {}
   void operator()(_Tp& __r) const { (__r.*_M_f)(); }
@@ -587,7 +587,7 @@
 };
 
 template <class _Tp>
-class const_mem_fun_ref_t<void, _Tp> : public unary_function<_Tp,void> {
+class const_mem_fun_ref_t<void, _Tp> : public unary_function<const _Tp &,void> {
 public:
   explicit const_mem_fun_ref_t(void (_Tp::*__pf)() const) : _M_f(__pf) {}
   void operator()(const _Tp& __r) const { (__r.*_M_f)(); }
@@ -616,7 +616,7 @@
 
 template <class _Tp, class _Arg>
 class mem_fun1_ref_t<void, _Tp, _Arg>
-  : public binary_function<_Tp,_Arg,void> {
+  : public binary_function<_Tp &,_Arg,void> {
 public:
   explicit mem_fun1_ref_t(void (_Tp::*__pf)(_Arg)) : _M_f(__pf) {}
   void operator()(_Tp& __r, _Arg __x) const { (__r.*_M_f)(__x); }
@@ -626,7 +626,7 @@
 
 template <class _Tp, class _Arg>
 class const_mem_fun1_ref_t<void, _Tp, _Arg>
-  : public binary_function<_Tp,_Arg,void> {
+  : public binary_function<const _Tp &,_Arg,void> {
 public:
   explicit const_mem_fun1_ref_t(void (_Tp::*__pf)(_Arg) const) : _M_f(__pf) {}
   void operator()(const _Tp& __r, _Arg __x) const { (__r.*_M_f)(__x); }