Boost C++ Libraries: Ticket #7343: Extend result_of to work with SFINAE

attachment set

Daniel Walker — Fri, 07 Sep 2012 03:28:24 GMT

attachment → djw_result_of_sfinae.patch

updated patch incorporating suggestions from Michel Morin

attachment set

Eric Niebler — Wed, 19 Sep 2012 17:26:27 GMT

attachment → ean_result_of_sfinae.patch

simpler patch, tested with clang-trunk

Eric Niebler — Wed, 19 Sep 2012 17:33:44 GMT

Daniel, I've attached a much simpler, lighter-weight patch. However, both of our patches are incomplete because of how result_of is defined:

// Uses declval following N3225 20.7.7.6 when F is not a pointer.
template<typename F BOOST_PP_COMMA_IF(BOOST_PP_ITERATION())
         BOOST_PP_ENUM_PARAMS(BOOST_PP_ITERATION(),typename T)>
struct result_of<F(BOOST_PP_ENUM_PARAMS(BOOST_PP_ITERATION(),T))>
    : mpl::if_<
          is_member_function_pointer<F>
        , detail::tr1_result_of_impl<
            typename remove_cv<F>::type,
            typename remove_cv<F>::type(BOOST_PP_ENUM_PARAMS(BOOST_PP_ITERATION(),T)), false
          >
        , detail::cpp0x_result_of_impl<
              F(BOOST_PP_ENUM_PARAMS(BOOST_PP_ITERATION(),T))
          >
      >::type
{};

If F is a member function pointer, the first branch of the mpl::if_ gets taken, and no clever SFINAE guards detail::tr1_result_of_impl.

But to a first approximation, I think this is good enough for 1.52. I tested my patch with clang-trunk, but not the most recent stable release, and not with msvc-11, which I'm downloading now. If you have access to those and want to get a jump on things, that'd be great. I'd like to get this change in to trunk ASAP so we have time to merge to release for 1.52.

Thanks for your work on this.

Daniel Walker — Wed, 19 Sep 2012 17:55:21 GMT

Thanks Eric. Your patch looks reasonable, and I like the simplifications. I'll test on clang 2.1 later today and assuming all's well I'll check your patch into trunk. I'd also like to see this in the 1.52 release. To my knowledge this is not a breaking change, i.e. enabling SFINAE doesn't invalidate any previously valid code, so we should be good to go.

Eric Niebler — Wed, 19 Sep 2012 20:47:11 GMT

Sadly, my patch doesn't work with msvc-11, but yours does. Unfortunately, yours has heavier TMP overhead. The best solution, IMO, would be to use yours for msvc-11 and use mine in other cases, assuming clang 3.1 can handle it. Thoughts?

Eric Niebler — Wed, 19 Sep 2012 21:06:30 GMT

For the record, here is the msvc bug that is blocking the simpler implementation from working:

https://connect.microsoft.com/VisualStudio/feedback/details/763618/too-eager-instantiation-in-class-template-partial-specialization

Michel Morin — Wed, 19 Sep 2012 22:01:11 GMT

I tested both versions (in a C++11 mode with BOOST_RESULT_OF_USE_DECLTYPE) and got the same results:

gcc 4.3-4.4: compilation error on sfinae_test
gcc 4.5-4.7, 4.8 (experimental): OK
clang 2.8-3.1, 3.2 (trunk): OK

I think gcc 4.3-4.4 had incomplete support of "SFINAE for expressions" (though BOOST_NO_SFINAE_EXPR is not defined on gcc 4.4).

Daniel Walker — Wed, 19 Sep 2012 23:17:47 GMT

I combined the two patches so that the thin implementation is the default and the is_callable implementation is a workaround for MSVC tested at 1700. I tested on clang 3.2 and gcc 4.7 with BOOST_RESULT_OF_USE_DECLTYPE and can corroborate Michel's results. Eric could you test on MSVC one more time? If everything is OK, I think its ready to merge to release.

Michel Morin — Wed, 19 Sep 2012 23:31:37 GMT

I tested r80605 (in a C++11 mode with BOOST_RESULT_OF_USE_DECLTYPE). The test results are the same as above:

gcc 4.3-4.4: compilation error on sfinae_test
gcc 4.5-4.7, 4.8 (experimental): OK
clang 2.8-3.1, 3.2 (trunk): OK

Eric Niebler — Thu, 20 Sep 2012 06:06:36 GMT

Tests pass on msvc 8-11. That does *not* mean we're ready to merge to release, though. It means we're ready to wait and see what crops up in the trunk regression reports. :-)

Michel Morin — Thu, 20 Sep 2012 12:09:00 GMT

I can see a tab in line 124 of result_of_iterate.hpp ;-)

Eric Niebler — Thu, 20 Sep 2012 17:10:10 GMT

Bah. My bad. Fixed in [80608].

Michel Morin — Fri, 21 Sep 2012 11:11:11 GMT

Eric, did you test on msvc 10 with BOOST_RESULT_OF_USE_DECLTYPE? (In other word, does the "Too-eager instantiation in class template partial specialization" bug happen only on msvc 11?)

Eric Niebler — Fri, 21 Sep 2012 18:57:16 GMT

Good question. No, it doesn't compile on msvc 10 in that configuration. I'll look into it.

Regardless, I took the liberty of merging all the outstanding result_of changes to release, since the trunk tests looked no worse than before. When I get a handle on the msvc regression, I'll merge that too. For reference, the release changeset is [80621].

Eric Niebler — Sat, 22 Sep 2012 19:18:57 GMT

Changeset [80636] on trunk changes the workaround for msvc and extends it to work for all compilers that don't do extended-sfinae-for-expressions. All the tests should pass now, even on gcc 4.3-4.4. Michel, can you give it a shot?

Daniel Walker — Sat, 22 Sep 2012 23:55:43 GMT

Works on gcc 4.4. Nice! However, BOOST_NO_SFINAE_EXPR does not appear to be defined for 4.4. boost/config/gcc.hpp has

#if GNUC < 4

(GNUC == 4 && GNUC_MINOR < 4)

# define BOOST_NO_SFINAE_EXPR #endif

Should that be GNUC_MINOR < 5?

Eric Niebler — Sun, 23 Sep 2012 00:16:49 GMT

I don't know. What's your reason for thinking that BOOST_NO_SFINAE_EXPR should be defined for gcc 4.4?

Daniel Walker — Sun, 23 Sep 2012 00:46:25 GMT

I don't know either, but you ifdefed the workaround with BOOST_NO_SFINAE_EXPR. I had to define it in order to test your changes, which do work on gcc 4.4. The tests fail on gcc 4.4 without the workaround.

Michel Morin — Sun, 23 Sep 2012 00:50:57 GMT

First, I did the following

Remove BOOST_RESULT_OF_USE_DECLTYPE guard for sfinae_test in result_of_test.cpp.
Make gcc 4.4 in a C++11 mode (with BOOST_RESULT_OF_USE_DECLTYPE) follow the BOOST_NO_SFINAE_EXPR branch.

Without applying 2, the test fails to compile on gcc 4.4 (in a C++11 mode BOOST_RESULT_OF_USE_DECLTYPE).

Here are the test results.

gcc 4.3 - 4.8 (in a C++03 mode): OK
gcc 4.3 - 4.8 (in a C++11 mode): OK

gcc 4.3 - 4.4 (in a C++11 mode + BOOST_RESULT_OF_USE_DECLTYPE): OK but with two warnings:

boost/utility/detail/result_of_iterate.hpp:100:
warning: object of type 'volatile int&' will not be accessed
in left-hand operand of comma

boost/utility/detail/result_of_iterate.hpp:100:
warning: object of type 'const volatile int&' will not be accessed
in left-hand operand of comma

gcc 4.5 - 4.8 (in a C++11 mode + BOOST_RESULT_OF_USE_DECLTYPE): OK

clang 2.8 - 3.2 (in a C++03 mode): OK
clang 2.8 - 3.2 (in a C++11 mode): OK
clang 2.8 - 3.2 (in a C++11 mode + BOOST_RESULT_OF_USE_DECLTYPE): OK

Michel Morin — Sun, 23 Sep 2012 00:51:44 GMT

Full warning messages on gcc 4.3-4.4 (in a C++11 mode BOOST_RESULT_OF_USE_DECLTYPE):

boost/utility/detail/result_of_iterate.hpp: In instantiation of 'const bool boost::detail::result_of_is_callable_2<result_of_member_function_template, volatile int&, int>::value':
boost/utility/detail/result_of_iterate.hpp:101:   instantiated from 'boost::detail::result_of_is_callable_2<result_of_member_function_template, volatile int&, int>'
boost/utility/enable_if.hpp:47:   instantiated from 'boost::lazy_enable_if<boost::detail::result_of_is_callable_2<result_of_member_function_template, volatile int&, int>, boost::detail::cpp0x_result_of_impl<result_of_member_function_template ()(volatile int&, int), false> >'
boost/utility/result_of.hpp:118:   instantiated from 'boost::detail::cpp0x_result_of_impl<result_of_member_function_template ()(volatile int&, int), true>'
boost/utility/result_of.hpp:59:   instantiated from 'boost::result_of<result_of_member_function_template ()(volatile int&, int)>'
untitled:245:   instantiated from here
boost/utility/detail/result_of_iterate.hpp:100: warning: object of type 'volatile int&' will not be accessed in left-hand operand of comma
boost/utility/detail/result_of_iterate.hpp: In instantiation of 'const bool boost::detail::result_of_is_callable_2<result_of_member_function_template, const volatile int&, int>::value':
boost/utility/detail/result_of_iterate.hpp:101:   instantiated from 'boost::detail::result_of_is_callable_2<result_of_member_function_template, const volatile int&, int>'
boost/utility/enable_if.hpp:47:   instantiated from 'boost::lazy_enable_if<boost::detail::result_of_is_callable_2<result_of_member_function_template, const volatile int&, int>, boost::detail::cpp0x_result_of_impl<result_of_member_function_template ()(const volatile int&, int), false> >'
boost/utility/result_of.hpp:118:   instantiated from 'boost::detail::cpp0x_result_of_impl<result_of_member_function_template ()(const volatile int&, int), true>'
boost/utility/result_of.hpp:59:   instantiated from 'boost::result_of<result_of_member_function_template ()(const volatile int&, int)>'
untitled:246:   instantiated from here
boost/utility/detail/result_of_iterate.hpp:100: warning: object of type 'const volatile int&' will not be accessed in left-hand operand of comma

Michel Morin — Sun, 23 Sep 2012 00:54:57 GMT

In the above warnings, untitled:*** means result_of_test.cpp:***. Sorry about that.

Eric Niebler — Sun, 23 Sep 2012 01:11:58 GMT

Changeset [80654] applies the workaround to gcc-4.4 also. Can you give it a shot? And how do I suppress errors on gcc?

EDIT: How do I suppress *WARNINGS* on gcc?

Michel Morin — Sun, 23 Sep 2012 01:47:50 GMT

gcc has

#pragma GCC diagnostic ignored "-Wxxxx"

but, before gcc 4.6, the following is not supported.

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wxxxx"
#pragma GCC diagnostic pop

So we cannot restore the user's warning conditions.

Another way to disable warnings is add

#pragma GCC system_header

Adding this pragma into result_of.hpp or result_of_iterate.hpp removes the warning.

Daniel Walker — Sun, 23 Sep 2012 01:51:53 GMT

Warnings suppressed in changeset [80655] on gcc. Thanks for all the work Eric. The users will appreciate it! And thanks for the help Michel.

Michel Morin — Sun, 23 Sep 2012 02:10:45 GMT

I tested r80655 on gcc and clang and it works fine.

Daniel, I think

#if defined(__GNUC__) && (__GNUC__ == 4) && (__GNUC_MINOR__ < 5) && defined(BOOST_RESULT_OF_USE_DECLTYPE)
#pragma GCC system_header
#endif

would be safer for the future development.

Eric Niebler — Sun, 23 Sep 2012 02:12:56 GMT

I found a workaround for the gcc warnings in code. Changeset [80656] makes the change and removes the #pragma GCC system_header.

Michel Morin — Sun, 23 Sep 2012 02:19:09 GMT

r80656 works fine on gcc and clang. Thanks Eric and Daniel for all the work!

component changed

Michel Morin — Tue, 25 Sep 2012 13:59:42 GMT

component utility → result_of