Bug 24803

Doug, can you look a bit into this one too? Thanks!

I don't know how to classify this. The basic problem is one that isn't
really solveable without rvalue references: you can't pass a literal
(e.g., 0) into the operator() of a reference_wrapper or any other
function object in TR1, because the arguments are accepted by
T&. Strictly speaking, this is not a bug. 

However, there are several ways that we could make this work, because
John is doing some very sensible things here [*]. Some ideas are:

  (1) Strong-arm a C++ front-end guru into implementing rvalue references,
      then use them in the TR1 function objects. Heck, we'll need 'em
      for C++0x anyway :)

  (2) When we know the argument types (e.g., when the T in reference_wrapper<T>
      is a function pointer, member function pointer, or class that is derived 
      from unary_function or binary_function), we could inject operator() 
      overloads that have precise type signatures, e.g.,

        T::result_type operator()(T::argument_type const&);

      That's really messy and would require quite a bit of refactoring in 
      reference_wrapper to get right.

  (3) Provide variants of operator() that accept "const T&" arguments in 
      addition to those that accept "T&" arguments. This makes 
      bind/mem_fn/reference_wrapper easier to use, but the number of overloads 
      blows up exponentially. Since we've already run into compilation time 
      issues, we'd have stop doing this once we hit four to five
      arguments (then we get the "I just added one more argument and
      everything blew up!" bug reports).

  (4) Strong-arm a C++ front-end guru into allowing the conversion
      operator (operator T&() const) to actually work for function
      call expressions, making most of reference_wrapper<T>::operator()
      magically go away. (Peter Dimov has suggested this before, but
      it hasn't really worked its way through the committee).


(1) is the forward-looking solution; (2) and (3) are hacks that could
make the code even uglier (ick!). (4) only fixes the problem for
reference_wrapper. At this point, I favor (1) strongly, but I'm not in
a position to strong-arm nor am I in a position to implement rvalue
references myself.

[*] One thing in the code that perhaps shouldn't work is passing the
literal 0 as the argument to 

  reference_wrapper<int (test_type::*)()>::operator()

That means that we need to be able to realize that it's a literal 0
(tricky, messy, but doable; see the construction-from-literal-zero
trick in tr1::function<>) and then convert that to a NULL test_type*.
I don't think John meant to test that, and I don't really think we
should care if it's broken.

A slightly simplified version of John's test case that does work
properly follows. Replace any of null_tt, null_ttc, or zero in the
calls to reference_wrapper objects to trigger compiler errors.

#include <tr1/functional>

struct test_type
{
   int member();
   int cmember()const;
   int member2(char);
   int cmember2(char)const;
};

struct functor1 : public std::unary_function<int, double>
{
   double operator()(int)const;
};

struct functor2 : public std::binary_function<int, char, double>
{
   double operator()(int, char)const;
};

template <class T>
void verify_return_type(T, T)
{
}

int main()
{
   test_type* null_tt = 0;
   const test_type* null_ttc = 0;
   int zero;

   std::tr1::reference_wrapper<double (int)>* pr1;
   verify_return_type((*pr1)(0), double());
   std::tr1::reference_wrapper<double (*)(int)>* pr2;
   verify_return_type((*pr2)(0), double());
   std::tr1::reference_wrapper<int (test_type::*)()>* pr3;
   verify_return_type((*pr3)(null_tt), int());
   std::tr1::reference_wrapper<int (test_type::*)()const>* pr4;
   verify_return_type((*pr4)(null_ttc), int());
   std::tr1::reference_wrapper<functor1>* pr5;
   verify_return_type((*pr5)(zero), double());

   std::tr1::reference_wrapper<double (int, char)>* pr1b;
   verify_return_type((*pr1b)(0,0), double());
   std::tr1::reference_wrapper<double (*)(int, char)>* pr2b;
   verify_return_type((*pr2b)(0,0), double());
   std::tr1::reference_wrapper<int (test_type::*)(char)>* pr3b;
   verify_return_type((*pr3b)(null_tt,zero), int());
   std::tr1::reference_wrapper<int (test_type::*)()const>* pr4b;
   verify_return_type((*pr4b)(null_ttc), int());
   std::tr1::reference_wrapper<functor2>* pr5b;
   verify_return_type((*pr5b)(zero,zero), double());
}

Hi Doug. First thing to do, before actually studying your extremely useful and
detailed reply (THANKS), is adding Howard in CC...

Doug's right: according to 3.4p4 passing an rvalue results in implementation defined behaviour.  So you can support it or not as you wish.  I'll update the Boost test case accordingly.

However... I predict that that I won't be the first to try and pass an rvalue through a call-wrapper :-)

Regards, John.

>   (1) Strong-arm a C++ front-end guru into implementing rvalue references,
>       then use them in the TR1 function objects. Heck, we'll need 'em
>       for C++0x anyway :)

At the risk of being reckless.  Yes...  well, more like YES!!!  ;-)

I know very little about gcc at this point.  But if the feature is implemented under a pragma (say rvalue_refs), default off, and if library code can test the pragma (at the preprocessor level), then we could really move forward (let the puns lie where they may).  N1855 went through core review in Mont Tremblant.  There were several good suggestions on rewording the proposed words, but absolutely nothing against the functionality or even the syntax.  That doesn't mean the syntax is set in concrete.  But it does mean the mud is firming up.  (I am guessing) It will take effort now for N1855 to not be accepted as is, or pretty darn close to it.  The lwg also voiced no complaints and indeed strong support for N1856 - N1862.  Almost irritation that I was taking up committee time with this stuff again (just do it!).

If the language and library work goes under a pragma, default off, we can do the following:

1.  Code like (for reference_wrapper):

#ifdef _GCC_MOVE
    operator()(T1&& t1) const {return (*data_)(std::forward<T1>(t1));}
#else
    operator()(T1& t1) const {return (*data_)(t1);}
#endif

2.  When bug reports like this come in, say "try turning on rvalue reference" (give instructions for doing that).

3.  If the syntax or functionality of the language changes, we are relatively free to change the compiler/lib, documenting as appropriate, since this is not default behavior, and should be documented as an extension - which could change in the future.

4.  When it becomes standard, we set the pragma to default on.

Dealing with bugs like this via hacks is more work than doing it right with rvalue reference.

So who are going to get?  I'd love to do it.  But I'm slightly worse than a newbie in this area.  I could write language tests for it though.  I also wouldn't mind following someone more experienced just for my own education.

How about Russell Yanofsky?

http://russ.hn.org/rref/

It would be best if one did not have to recompile libstdc++ when flipping this pragma.  In order to achieve that you have to be committed to keeping as much out of the binary lib as possible.  To date the direction of libstdc++ has been just the opposite.

Ok, therefore suspended (not really invalid, not really open) seems to me an
appropriate status. Otherwise, a mildly "depressing" remark: I'm pretty sure
some people are not very happy with pragmas. I'm pretty sure I read something
to that effect even specifically about rvalue references... But let's discuss
the thing, of course. About thar Russell Yanofsky, no news lately, I read
*months* ago about his willingness to implement rvalue references, then nothing
more happened.

Ok, let's suspend it, for now.

Hi Doug. Wanted to start updating parts of <functional>, and, as expected, with straightforward rvalue reference changes I can compile the functior1 case. However, I'm not clear about the int (test_type::*)() case, which is indeed failing: can you help me a bit more finding the place where function deals with literal-zero?

Created attachment 16625 [details]
update result_of and reference_wrapper

In the current draft, reference_wrapper invocation requires Callable, which means it should not work for pointers to members. The same is true of result_of, which determines the result of an expression that isn't valid syntax for pointers to members.

That said, here's a patch that updates result_of and reference_wrapper to work for pointers to members as well as callable types, using result_of in place of the Callable concept.  If result_of and reference_wrapper really shouldn't work for pointers to members then this patch would be much simpler, and there's no need for __invoke() either.

(In reply to comment #9)
> That said, here's a patch that updates result_of and reference_wrapper to work
> for pointers to members as well as callable types, using result_of in place of
> the Callable concept.  If result_of and reference_wrapper really shouldn't work
> for pointers to members then this patch would be much simpler, and there's no
> need for __invoke() either.

Excellent Jon. Please, commit the patch and let's close the PR for 4.4. A couple of minor nits, if you want: you can remove the main of result_of_neg, being a compile-only testcase; also I think a commented-out VERIFY in result_of can be just removed entirely. Thanks again! (let me know if for some reason you have trouble committing)

Subject: Bug 24803

Author: redi
Date: Wed Nov  5 02:05:59 2008
New Revision: 141596

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=141596
Log:
	PR libstdc++/24803
	PR libstdc++/37351
	* include/tr1_impl/functional (result_of,reference_wrapper): Update
	to use C++0x features.
	* testsuite/20_util/function_objects/return_types/result_of.cc: New.
	* testsuite/20_util/function_objects/return_types/result_of_neg.cc:
	New.
	* testsuite/20_util/function_objects/return_types/result_of_ref.cc:
	New.
	* testsuite/20_util/reference_wrapper/24803.cc: Uncomment FIXMEs.

Added:
    trunk/libstdc++-v3/testsuite/20_util/function_objects/return_types/
    trunk/libstdc++-v3/testsuite/20_util/function_objects/return_types/result_of.cc
    trunk/libstdc++-v3/testsuite/20_util/function_objects/return_types/result_of_neg.cc
    trunk/libstdc++-v3/testsuite/20_util/function_objects/return_types/result_of_ref.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/tr1_impl/functional
    trunk/libstdc++-v3/testsuite/20_util/reference_wrapper/24803.cc

Fixed for 4.4 (including the nits you spotted, thanks Paolo)

argh! I've broken call_once, re-opening while I look into it.  A testsuite_files was causing some tests not to run.

the problem is due to bug 35569 - I'll have to revert my patch until I can fix that properly

(In reply to comment #14)
> the problem is due to bug 35569 - I'll have to revert my patch until I can fix
> that properly

Argh, too bad. Yes, please revert for now and close libstdc++/38017.

(In reply to comment #9)
> 
> In the current draft, reference_wrapper invocation requires Callable, which
> means it should not work for pointers to members. The same is true of
> result_of, which determines the result of an expression that isn't valid syntax
> for pointers to members.

Could Howard or Doug comment on this?  the Callable concept and result_of both determine the result of an expression  fn(args...)  which in invalid if fn is a pointer to member.

If that's correct, this bug could be closed as INVALID, but that would mean std::bind and std::reference_wrapper do not support features that work with the equivalent components in TR1.  It also means that std::bind<R>() works with pointers to members, even though std::bind() doesn't, because the former doesn't rely on Callable and is only defined in terms of INVOKE which does support pointers to members.


I'm trying to figure out if I need to continue to support this behaviour, or if I can simplify result_of by ignoring pointers to members, but I think this should be an LWG issue.

At the moment, not actively working on this...

Subject: Bug 24803

Author: redi
Date: Tue Jan 12 00:53:30 2010
New Revision: 155826

URL: http://gcc.gnu.org/viewcvs?root=gcc&view=rev&rev=155826
Log:
2010-01-12  Jonathan Wakely  <jwakely.gcc@gmail.com>

	PR libstdc++/24803
	PR libstdc++/35569
	PR libstdc++/42593
	* include/std/functional (bind): Forward rvalues and detect correct
	result type of bound function object.
	* include/std/mutex (call_once): Specify bind result type.
	* testsuite/20_util/reference_wrapper/invoke.cc: Remove invalid tests.
	* testsuite/20_util/reference_wrapper/24803.cc: Remove invalid tests
	and enable FIXME tests.
	* testsuite/20_util/bind/35569.cc: New.
	* testsuite/20_util/bind/ref2.cc: New.
	* testsuite/20_util/bind/38889.cc: New.
	* testsuite/20_util/bind/ref_neg.cc: New.
	* testsuite/20_util/bind/42593.cc: New.


Added:
    trunk/libstdc++-v3/testsuite/20_util/bind/35569.cc
    trunk/libstdc++-v3/testsuite/20_util/bind/38889.cc
    trunk/libstdc++-v3/testsuite/20_util/bind/42593.cc
    trunk/libstdc++-v3/testsuite/20_util/bind/ref2.cc
    trunk/libstdc++-v3/testsuite/20_util/bind/ref_neg.cc
Modified:
    trunk/libstdc++-v3/ChangeLog
    trunk/libstdc++-v3/include/std/functional
    trunk/libstdc++-v3/include/std/mutex
    trunk/libstdc++-v3/testsuite/20_util/reference_wrapper/24803.cc
    trunk/libstdc++-v3/testsuite/20_util/reference_wrapper/invoke.cc

As I said earlier in this audit trail, the current draft defines the call operator in terms of std::result_of and so it doesn't support invoking a pointer-to-member. I have removed the relevant bits of the 24803.cc testcase, and enabled the FIXME parts that needed rvalue reference support, which work now that std::reference_wrapper::operator() uses perfect forwarding.
 
I think this bug could be closed now, any objections?

(In reply to comment #19)
> I think this bug could be closed now, any objections?

Not from me, I agree it can be closed. I'd like only to make sure we don't forget about the issue with pointers to member functions: is there a DR open already? Maybe we could also find a way to mention it in the code, if you haven't done that already, didn't check.

(In reply to comment #20)
> I'd like only to make sure we don't
> forget about the issue with pointers to member functions: is there a DR open
> already? 

It's been raised on the lib reflector byme, Stephan T. Lavavej and Alisdair Meredith.  I'm not sufficiently motivated to open an issue for it, there are workarounds using mem_fn.

The proposed resolution for LWG 1225 reinforces my interpretation that result_of does not support pointers-to-members.

> Maybe we could also find a way to mention it in the code, if you
> haven't done that already, didn't check.
 
No, I didn't mention it in the code.

Interesting... lately people (*) are apparently happy with all sorts of regressions wrt C++98/TR1...

Anyway, then let's close it, actually I leave to you the choice between fixed, wontfix and invalid ;)

(*) Stephan at least is also happy with std::vector & co not explicitly instantiable anymore for non-DefaultConstructible types: if I remember all the time I spent with Matt Austern encouragement to tweak all our containers for that... Agreed, the new philosophy of adding requirements to the specific members allows more flexibility together with non-moveable types, etc, but in my opinion it really makes sense only together with Concepts where you can elegantly make a member "disappear" depending on a Require clause... bah...

Ok, for now let's close this as WONTFIX. At your ease, Jon, it would be nice if you could add the # of the reflector message where you raised the issue...

Alisdair asked about it most recently in c++std-lib-24833, Stephan asked before that in c++std-lib-23007

Also related, n1695 proposed making pointers-to-members callable, Doug demonstrated a way to do it with concept maps in c++std-lib-21092