llvm / llvm-project

The LLVM Project is a collection of modular and reusable compiler and toolchain technologies.
http://llvm.org
Other
29.36k stars 12.14k forks source link

Regression libc++: Usage of std::ref compiles in C++17 but not C++20 #61866

Open PixelRick opened 1 year ago

PixelRick commented 1 year ago

Tested with Clang 16.0.0.

Minimal example:

//#undef __cplusplus
//#define __cplusplus 201703 // compiles when forcing libc++ as if compiling under C++17
#include <functional>
#include <iostream>
#include <type_traits>

template<class Fun>
class y_combinator_result {
    Fun fun_;
public:
    template<class T>
    explicit y_combinator_result(T &&fun): fun_(std::forward<T>(fun)) {}

    template<class ...Args>
    decltype(auto) operator()(Args &&...args) {
        return fun_(std::ref(*this), std::forward<Args>(args)...);
    }
};

template<class Fun>
decltype(auto) y_combinator(Fun &&fun) {
    return y_combinator_result<std::decay_t<Fun>>(std::forward<Fun>(fun));
}

int main() 
{
    using Type = int; // Using an other integer type such as "unsigned int" makes the code compile

    auto almost_factorial = [](auto factorial, Type n) -> Type {
        if (n == 0) {
            return static_cast<Type>(1); 
        } else {
            return static_cast<Type>(n * factorial(n-1));
        }
    };
    auto factorial = y_combinator(std::ref(almost_factorial)); // Not using std::ref makes the code compile

    //std::cout << factorial(5u) << std::endl; // Uncommenting this line makes the code compile
    std::cout << factorial(5) << std::endl; // Does not compile without previous line

    return 0;
}

Compilation output on cpp.sh+%7B%7D%0D%0A%0D%0A++++template%3Cclass+...Args%3E%0D%0A++++decltype(auto)+operator()(Args+%26%26...args)+%7B%0D%0A++++++++return+fun_(std%3A%3Aref(this)%2C+std%3A%3Aforward%3CArgs%3E(args)...)%3B%0D%0A++++%7D%0D%0A%7D%3B%0D%0A%0D%0Atemplate%3Cclass+Fun%3E%0D%0Adecltype(auto)+y_combinator(Fun+%26%26fun)+%7B%0D%0A++++return+y_combinator_result%3Cstd%3A%3Adecay_t%3CFun%3E%3E(std%3A%3Aforward%3CFun%3E(fun))%3B%0D%0A%7D%0D%0A%0D%0Aint+main()+%0D%0A%7B%0D%0A++++using+Type+%3D+int%3B+%2F%2F+Using+an+other+integer+type+such+as+%22unsigned+int%22+makes+the+code+compile%0D%0A++++%0D%0A++++auto+almost_factorial+%3D+%5B%5D(auto+factorial%2C+Type+n)+-%3E+Type+%7B%0D%0A++++++++if+(n+%3D%3D+0)+%7B%0D%0A++++++++++++return+static_cast%3CType%3E(1)%3B+%0D%0A++++++++%7D+else+%7B%0D%0A++++++++++++return+static_cast%3CType%3E(n++factorial(n-1))%3B%0D%0A++++++++%7D%0D%0A++++%7D%3B%0D%0A++++auto+factorial+%3D+y_combinator(std%3A%3Aref(almost_factorial))%3B+%2F%2F+Not+using+std%3A%3Aref+makes+the+code+compile%0D%0A++++%0D%0A++++%2F%2Fstd%3A%3Acout+%3C%3C+factorial(5u)+%3C%3C+std%3A%3Aendl%3B+%2F%2F+Uncommenting+this+line+makes+the+code+compile%0D%0A++++std%3A%3Acout+%3C%3C+factorial(5)+%3C%3C+std%3A%3Aendl%3B+%2F%2F+Does+not+compile+without+previous+line%0D%0A++++%0D%0A++++return+0%3B%0D%0A%7D):

main.cpp:33:42: error: no matching function for call to object of type 'std::reference_wrapper<y_combinator_result<std::reference_wrapper<(lambda at main.cpp:29:29)>>>'
            return static_cast<Type>(n * factorial(n-1));
                                         ^~~~~~~~~
/root/emsdk/upstream/emscripten/cache/sysroot/include/c++/v1/type_traits:3640:23: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<std::reference_wrapper<y_combinator_result<std::reference_wrapper<(lambda at main.cpp:29:29)>>>>' requested here
    { return          static_cast<_Fp&&>(__f)(static_cast<_Args&&>(__args)...); }
                      ^
/root/emsdk/upstream/emscripten/cache/sysroot/include/c++/v1/__functional/reference_wrapper.h:60:23: note: in instantiation of function template specialization 'std::__invoke<(lambda at main.cpp:29:29) &, std::reference_wrapper<y_combinator_result<std::reference_wrapper<(lambda at main.cpp:29:29)>>>, int>' requested here
        return _VSTD::__invoke(get(), _VSTD::forward<_ArgTypes>(__args)...);
                      ^
main.cpp:16:16: note: in instantiation of function template specialization 'std::reference_wrapper<(lambda at main.cpp:29:29)>::operator()<std::reference_wrapper<y_combinator_result<std::reference_wrapper<(lambda at main.cpp:29:29)>>>, int>' requested here
        return fun_(std::ref(*this), std::forward<Args>(args)...);
               ^
main.cpp:39:27: note: in instantiation of function template specialization 'y_combinator_result<std::reference_wrapper<(lambda at main.cpp:29:29)>>::operator()<int>' requested here
    std::cout << factorial(5) << std::endl; // Does not compile without previous line
                          ^
/root/emsdk/upstream/emscripten/cache/sysroot/include/c++/v1/__functional/reference_wrapper.h:59:5: note: candidate template ignored: substitution failure [with _ArgTypes = <int>]: no type named 'type' in 'std::__invoke_of<y_combinator_result<std::reference_wrapper<(lambda at main.cpp:29:29)>> &, int>'
    operator() (_ArgTypes&&... __args) const {
    ^
1 error generated.

maybe related to #43743

philnik777 commented 1 year ago

This has nothing to do with libc++: https://godbolt.org/z/bdbbMxcab. This looks like clang rejects valid code.

llvmbot commented 1 year ago

@llvm/issue-subscribers-clang-frontend

shafik commented 1 year ago

@erichkeane @royjacobson I believe clang is incorrectly rejecting this code but it is not obvious to me what is going on here, any insights?

erichkeane commented 1 year ago

I have no idea, that is really bizarre. Things that should have nothing to do with that lookup having an effect is confusing to me (like, why does the type of the value passed into the non templated argument matter?).

I'd need something more reduced however (without #include!) to have a better idea though.

PixelRick commented 1 year ago

@erichkeane Here is a simplified example without includes and which is still rejected: (edit: "5u"->"5")

template<typename T>
struct ref_wrap {
    T* ptr;

    constexpr ref_wrap(T& t)
        : ptr(&t) {}

    template<class... ArgTypes>
    constexpr auto operator() (ArgTypes... args) const
    {
        return (*ptr)(args...);
    }
};
template<class T>
ref_wrap(T&) -> ref_wrap<T>;

template<class Fun>
class y_combinator_result {
    Fun fun_;
public:
    template<class T>
    explicit y_combinator_result(T fun): fun_(fun) {}

    template<class ...Args>
    decltype(auto) operator()(Args...args) {
        return fun_(ref_wrap(*this), args...);
    }
};

template<class Fun>
decltype(auto) y_combinator(Fun fun) {
    return y_combinator_result<Fun>(fun);
}

int main() 
{
    using Type = int; // Using an other integer type such as "unsigned int" makes the code compile

    auto almost_factorial = [](auto factorial, Type n) -> Type {
        if (n == 0) {
            return static_cast<Type>(1); 
        } else {
            return static_cast<Type>(n * factorial(n-1));
        }
    };
    auto factorial = y_combinator(ref_wrap(almost_factorial)); // Not using ref_wrap makes the code compile

    int a = 0;
    //a += factorial(5u); // Uncommenting this line makes the code compile
    a += factorial(5); // Does not compile without previous line

    return a;
}

The error is different:

main.cpp:11:16: error: function 'operator()<int>' with deduced return type cannot be used before it is defined
        return (*ptr)(args...);
               ^
main.cpp:43:42: note: in instantiation of function template specialization 'ref_wrap<y_combinator_result<ref_wrap<(lambda at main.cpp:39:29)>>>::operator()<int>' requested here
            return static_cast<Type>(n * factorial(n-1));
                                         ^
main.cpp:11:16: note: in instantiation of function template specialization 'main()::(anonymous class)::operator()<ref_wrap<y_combinator_result<ref_wrap<(lambda at main.cpp:39:29)>>>>' requested here
        return (*ptr)(args...);
               ^
main.cpp:26:16: note: in instantiation of function template specialization 'ref_wrap<(lambda at main.cpp:39:29)>::operator()<ref_wrap<y_combinator_result<ref_wrap<(lambda at main.cpp:39:29)>>>, int>' requested here
        return fun_(ref_wrap(*this), args...);
               ^
main.cpp:50:19: note: in instantiation of function template specialization 'y_combinator_result<ref_wrap<(lambda at main.cpp:39:29)>>::operator()<int>' requested here
    a += factorial(5); // Does not compile without previous line
                  ^
main.cpp:25:20: note: 'operator()<int>' declared here
    decltype(auto) operator()(Args...args) {
                   ^
1 error generated.
erichkeane commented 1 year ago

So that copy seems to also fail in MSVC for the same reason: https://godbolt.org/z/snErzTr3G

(note that I had to change the 5u at the bottom to a 5, else everyone correctly compiled it).

PixelRick commented 1 year ago

Indeed, it is so strange that MSVC now also fails with "5" but accepts it as Clang with "5u". As if MSVC and Clang shared some implementation details.

erichkeane commented 1 year ago

I suspect it is that Clang/MSVC are sharing the same status of implementing a certain defect-report. But I dont' know enough to know what is happening with it here. BUT the issue is basically that the code example causes a cycle in trying to figure out return types.

y_combinator_result::operator()'s deduced return type is the type of the ref_wrapper::operator(), which is the return type of y_combinator_result::operator().

The type passed in matters, as it is only a problem if the type of the argument to call a+ = factorial matches that of the type defined in Type (so if they are both unsigned, than the problem comes back!). So there must be a cycle that gets broken if they are different types.

EDIT: I THINK the issue is a cycle, but I may have missed a step above with the lambda, but i think the cycle issue still stands.

PixelRick commented 1 year ago

Using std::invoke_result_t<Fun, ref_wrap<y_combinator_result>&, Args...> instead of decltype(auto) there makes the code compile with all 3 compilers. I used the y combinator implementation given in the y_combinator proposal so the question is: is the code non-valid and compilers do incorrectly allow it in certain cases, or is the code valid but subject to defect ?