Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libc++] std::invoke() substitution failure when using function object with default argument #106428

Closed
tcbrindle opened this issue Aug 28, 2024 · 4 comments · Fixed by #106925
Closed
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"

Comments

@tcbrindle
Copy link

The following test program compiles with libc++17 and libstdc++, but fails with libc++18:

#include <concepts>
#include <functional>
#include <ranges>
#include <source_location>

#define FWD(x) static_cast<decltype(x)&&>(x)

struct add_fn {
    template <std::integral T>
    constexpr auto operator()(T lhs, T rhs,
                              std::source_location loc = std::source_location::current())
        const -> T
    {
        return lhs + rhs;
    }
};

inline constexpr auto add = add_fn{};

struct fold_op {
    template <typename Rng, typename Func, typename Init>
    constexpr auto operator()(Rng&& rng, Func func, Init init) const
    {
        auto init_ = std::ranges::range_value_t<Rng>(std::move(init));
        for (auto&& elem : rng) {
            init_ = std::invoke(func, std::move(init_), FWD(elem));
        }
        return init_;
    }
};

inline constexpr auto fold = fold_op{};


auto sum(std::span<int const> arr) -> int
{
    return fold(arr, add, 0);
}

https://godbolt.org/z/h47dnM3eP

The error is:

<source>:27:21: error: no matching function for call to 'invoke'
   27 |             init_ = std::invoke(func, std::move(init_), FWD(elem));
      |                     ^~~~~~~~~~~
<source>:38:16: note: in instantiation of function template specialization 'fold_op::operator()<std::span<const int> &, add_fn, int>' requested here
   38 |     return fold(arr, add, 0);
      |                ^
/opt/compiler-explorer/clang-18.1.0/bin/../include/c++/v1/__functional/invoke.h:27:1: note: candidate template ignored: substitution failure [with _Fn = add_fn &, _Args = <__libcpp_remove_reference_t<int &>, const int &>]
   27 | invoke(_Fn&& __f, _Args&&... __args) noexcept(is_nothrow_invocable_v<_Fn, _Args...>) {
      | ^
1 error generated.
Compiler returned: 1

Removing the source_location default argument from add_fn::operator() allows the code to compile as expected.

@github-actions github-actions bot added the libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. label Aug 28, 2024
tcbrindle added a commit to tcbrindle/flux that referenced this issue Aug 28, 2024
It seems like std::invoke() in libc++18 (but not previous versions) doesn't like default function parameters in certain contexts.

Reported as llvm/llvm-project#106428
@MitalAshok
Copy link
Contributor

This appears to be a Clang regression in Clang 18, not in libc++.

Reduced: https://godbolt.org/z/TMc3Wqjb8

template <class _Fp, class... _Args>
decltype(_Fp{}(0, 0))
__invoke(_Fp&& __f);

template<typename T>
struct type_identity { using type = T; };

template<class Fn>
struct invoke_result : type_identity<decltype(__invoke(Fn{}))> {};

namespace std {
struct source_location {
  struct __impl {
    const char *_M_file_name;
    const char *_M_function_name;
    unsigned _M_line;
    unsigned _M_column;
  };
};
}

struct add_fn {
    template <typename T>
    constexpr auto operator()(T lhs, T rhs,
                              const std::source_location::__impl* = __builtin_source_location())
        const -> T
    {
        return lhs + rhs;
    }
};

using i = invoke_result<add_fn>::type;
static_assert(__is_same(i, int));
<source>:39:25: error: unknown type name 'i'
   34 | static_assert(__is_same(i, int));
      |                         ^

Started with 8c2b0d4 / #78436 (CC @cor3ntin)

@ldionne ldionne added clang Clang issues not falling into any other category and removed libc++ libc++ C++ Standard Library. Not GNU libstdc++. Not libc++abi. labels Aug 29, 2024
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 1, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they
are first used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template argumente
 - They morally depend on the context in which they are used
   (rather than called from).

It's fair to say that this is quite novels and confuses
clang. In particular, in some cases, we used to create
dependent SourceLocExpr and never subsequently transform them,
leaving dependent objects in instantiated functions types.
To work around that we avoid replacing SourceLocExpr when we think
they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs,
and seem to only affect scenarios in which the value of the
SourceLocExpr does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 1, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they
are first used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template argumente
 - They morally depend on the context in which they are used
   (rather than called from).

It's fair to say that this is quite novels and confuses
clang. In particular, in some cases, we used to create
dependent SourceLocExpr and never subsequently transform them,
leaving dependent objects in instantiated functions types.
To work around that we avoid replacing SourceLocExpr when we think
they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs,
and seem to only affect scenarios in which the value of the
SourceLocExpr does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 2, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they
are first used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template argumente
 - They morally depend on the context in which they are used
   (rather than called from).

It's fair to say that this is quite novels and confuses
clang. In particular, in some cases, we used to create
dependent SourceLocExpr and never subsequently transform them,
leaving dependent objects in instantiated functions types.
To work around that we avoid replacing SourceLocExpr when we think
they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs,
and seem to only affect scenarios in which the value of the
SourceLocExpr does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
cor3ntin added a commit to cor3ntin/llvm-project that referenced this issue Sep 4, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they are first
used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template arguments
- They morally depend on the context in which they are used (rather than
called from).

It's fair to say that this is quite novels and confuses clang. In
particular, in some cases, we used to create dependent SourceLocExpr and
never subsequently transform them, leaving dependent objects in
instantiated functions types. To work around that we avoid replacing
SourceLocExpr when we think they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs, and
seem to only affect scenarios in which the value of the SourceLocExpr
does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
Fixes llvm#80210
Fixes llvm#85373

---------

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
@EugeneZelenko EugeneZelenko added clang:frontend Language frontend issues, e.g. anything involving "Sema" and removed clang Clang issues not falling into any other category labels Sep 4, 2024
@llvmbot
Copy link
Member

llvmbot commented Sep 4, 2024

@llvm/issue-subscribers-clang-frontend

Author: Tristan Brindle (tcbrindle)

The following test program compiles with libc++17 and libstdc++, but fails with libc++18:
#include &lt;concepts&gt;
#include &lt;functional&gt;
#include &lt;ranges&gt;
#include &lt;source_location&gt;

#define FWD(x) static_cast&lt;decltype(x)&amp;&amp;&gt;(x)

struct add_fn {
    template &lt;std::integral T&gt;
    constexpr auto operator()(T lhs, T rhs,
                              std::source_location loc = std::source_location::current())
        const -&gt; T
    {
        return lhs + rhs;
    }
};

inline constexpr auto add = add_fn{};

struct fold_op {
    template &lt;typename Rng, typename Func, typename Init&gt;
    constexpr auto operator()(Rng&amp;&amp; rng, Func func, Init init) const
    {
        auto init_ = std::ranges::range_value_t&lt;Rng&gt;(std::move(init));
        for (auto&amp;&amp; elem : rng) {
            init_ = std::invoke(func, std::move(init_), FWD(elem));
        }
        return init_;
    }
};

inline constexpr auto fold = fold_op{};


auto sum(std::span&lt;int const&gt; arr) -&gt; int
{
    return fold(arr, add, 0);
}

https://godbolt.org/z/h47dnM3eP

The error is:

&lt;source&gt;:27:21: error: no matching function for call to 'invoke'
   27 |             init_ = std::invoke(func, std::move(init_), FWD(elem));
      |                     ^~~~~~~~~~~
&lt;source&gt;:38:16: note: in instantiation of function template specialization 'fold_op::operator()&lt;std::span&lt;const int&gt; &amp;, add_fn, int&gt;' requested here
   38 |     return fold(arr, add, 0);
      |                ^
/opt/compiler-explorer/clang-18.1.0/bin/../include/c++/v1/__functional/invoke.h:27:1: note: candidate template ignored: substitution failure [with _Fn = add_fn &amp;, _Args = &lt;__libcpp_remove_reference_t&lt;int &amp;&gt;, const int &amp;&gt;]
   27 | invoke(_Fn&amp;&amp; __f, _Args&amp;&amp;... __args) noexcept(is_nothrow_invocable_v&lt;_Fn, _Args...&gt;) {
      | ^
1 error generated.
Compiler returned: 1

Removing the source_location default argument from add_fn::operator() allows the code to compile as expected.

tru pushed a commit to cor3ntin/llvm-project that referenced this issue Sep 10, 2024
In llvm#78436 we made some SourceLocExpr dependent to
deal with the fact that their value should reflect the name of
specialized function - rather than the rtemplate in which they are first
used.

However SourceLocExpr are unusual in two ways
 - They don't depend on template arguments
- They morally depend on the context in which they are used (rather than
called from).

It's fair to say that this is quite novels and confuses clang. In
particular, in some cases, we used to create dependent SourceLocExpr and
never subsequently transform them, leaving dependent objects in
instantiated functions types. To work around that we avoid replacing
SourceLocExpr when we think they could remain dependent.
It's certainly not perfect but it fixes a number of reported bugs, and
seem to only affect scenarios in which the value of the SourceLocExpr
does not matter (overload resolution).

Fixes llvm#106428
Fixes llvm#81155
Fixes llvm#80210
Fixes llvm#85373

---------

Co-authored-by: Aaron Ballman <aaron@aaronballman.com>
@DNKpp
Copy link

DNKpp commented Sep 25, 2024

Will there ever be a patch for clang-18? Or will this be broken for ever for this whole major version?

@philnik777
Copy link
Contributor

@DNKpp We're in the LLVM 20 release cycle now, so fixes will only be back-ported to LLVM 19.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema"
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants