Naios / function2

Improved and configurable drop-in replacement to std::function that supports move only types, multiple overloads and more
http://naios.github.io/function2
Boost Software License 1.0
539 stars 47 forks source link

Got MSVC error C2248 when I use unique_function with boost::callable_traits::is_invocable #54

Closed redboltz closed 1 year ago

redboltz commented 1 year ago

@Naios

When I use fu2::unique_function with boost::callable_traits::is_invocable on msvc v19.33 or older (on godbolt) , then I got C2248 error. It doesn't happen with std::function. When I test it on msvc latest (on godbolt), no error happens. When I test it on g++ and clang++, no error happens.

It might be msvc issue, but msvc v19.33 is not so old. If fu2::unique_function could avoid the issue, it is very helpful.


Commit Hash

2d3a878ef19dd5d2fb188898513610fac0a48621

Expected Behavior

Build finished without the error.

Actual Behavior

Got C2248 (cannot access private member) error.

Steps to Reproduce

#include <function2/function2.hpp>
#include <boost/callable_traits.hpp>
#include <type_traits>

using std_func_t = std::function<void()>;
using unique_func_t = fu2::unique_function<void()>;

int main() {
    // std::function : no error
    static_assert(boost::callable_traits::is_invocable<std_func_t>::value);
    // fu2::unique_function : error C2248 on msvc v19.33 and older
    static_assert(boost::callable_traits::is_invocable<unique_func_t>::value);
}
Godbolt link Compiler Link Result
msvc v19.33 https://godbolt.org/z/PxMa4q3oo error C2248
msvc latest https://godbolt.org/z/bbrxb6nTe no error
clang++ 15.0.0 https://godbolt.org/z/zPEGqbjsn no error
g++ 12.2 https://godbolt.org/z/7Eb1hs8KW no error

Your Environment

redboltz commented 1 year ago

I noticed that when I replace boost::callable_traits::is_invocable with std::is_invocable (since C++17), then the error is disappeared.

https://godbolt.org/z/vM19nhfv7

It seems that the issue is caused by boost::callable_traits.

But still unclear for me.

I reported the issue to boost::callable_traits github too.

Naios commented 1 year ago

I would not count on boost::callable_traits::is_invocable, they even specialize for std::function for whichever reason:

template<typename F>
    struct test_invoke<function<F>, true /*abominable*/> {
        auto operator()(...) const -> substitution_failure;
    };

https://github.com/boostorg/callable_traits/blob/master/include/boost/callable_traits/detail/is_invocable_impl.hpp

or

    // Here is where the magic happens
    template<typename T>
    using traits = typename BOOST_CLBL_TRTS_DISJUNCTION(
        function_object<unwrap_reference<T>>,
        function<T>,
        pmf<T>,
        pmd<T>,
        default_callable_traits<T>
    )::traits;

https://github.com/boostorg/callable_traits/blob/master/include/boost/callable_traits/detail/traits.hpp

This is simply not correct for a callable trait to specialize on a particular implementation.

My guess (without further looking into it) is that they try to access the operator() to the function2 object by member function pointer or similar to trigger the issue. The issue goes away if we inherit the operator_impl with public. However, this won't be changed because it is critical to prevent artificial padding to the function object by inheritence.

If you can show that this is a MSVC compiler issue and caused by the library, I might add a workaround (if it is zero cost), otherwise this issue is out of scope for this library and you could use a correct is_invocable implementation polyfill other than boost.

redboltz commented 1 year ago

Thanks you the response. I will use std::invocable on C++17 and use some workaround on C++14. My project requires C++14 or later, so it is enough for me.

Also I've checked _MSC_FULL_VER.

_MSC_VER _MSC_FULL_VER result
1934 193431921 success
1933 193331631 C2248 error

I will check the release note and developper community. If I would find the compiler bug fix that is related to the issue, I will report here.

badair commented 1 year ago

I would not count on boost::callable_traits::is_invocable, they even specialize for std::function for whichever reason: This is simply not correct for a callable trait to specialize on a particular implementation.

For the record, it's definitely not specializing for std::function. That's something in the private details namespace.

My guess (without further looking into it) is that they try to access the operator() to the function2 object by member function pointer

That is correct, it's doing this:

template<typename T>
struct has_normal_call_operator
{
    template<typename N, N Value>
    struct check { check(std::nullptr_t) {} };

    template<typename U>
    static std::int8_t test(
        check<decltype(&U::operator()), &U::operator()>);

    template<typename>
    static std::int16_t test(...);

    static constexpr bool value =
        sizeof(test<T>(nullptr)) == sizeof(std::int8_t);
};

Interestingly, the case in question works for GCC and Clang, and ICEs an older version of MSVC. I haven't seen this case before. Anyway, yes, definitely use std::is_invocable when possible.

Naios commented 1 year ago

@badair Thank you for pointing this out.

Sadly it is impossible to work around this issue in this library. For multiple function signatures me must inherit from multiple objects. Also the inheritence must be private in order to make empty object optimization work.

Probably this could be fixed in the boost callable trait by just testing the expression decltype(callable(args...)) as std::is_invoceable is doing it.

Because of above reasons I'm closing this issue.