ThePhD / sol2

Sol3 (sol2 v3.0) - a C++ <-> Lua API wrapper with advanced features and top notch performance - is here, and it's great! Documentation:
http://sol2.rtfd.io/
MIT License
4.22k stars 517 forks source link

Doesn't build with Clang 18.1.0 #1581

Open checkraisefold opened 8 months ago

checkraisefold commented 8 months ago

On the latest major release (candidate) of Clang, sol2 flat out doesn't compile with a fatal error and spits out a bunch of warnings. https://github.com/jpxs-intl/RosaServer/actions/runs/7929791195/job/21650728959 for the run logs, or here:

/home/runner/work/RosaServer/RosaServer/sol2/include/sol/function_types.hpp:110:31: fatal error: address of overloaded function 'call' does not match required type 'int (lua_State *)'
  110 |                                 lua_CFunction freefunc = &function_detail::upvalue_this_member_variable<C, Fx>::template call<is_yielding, no_trampoline>;
      |                                                           ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/function_types.hpp:268:5: note: in instantiation of function template specialization 'sol::function_detail::select_member_variable<false, false, float Vector::*>' requested here
  268 |                                 select_member_variable<is_yielding, no_trampoline>(L, std::forward<Fx>(fx), std::forward<Args>(args)...);
      |                                 ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/function_types.hpp:392:22: note: in instantiation of function template specialization 'sol::function_detail::select<false, false, float Vector::*>' requested here
  392 |                                 function_detail::select<false, false>(L, std::forward<Args>(args)...);
      |                                                  ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/stack_core.hpp:878:14: note: in instantiation of function template specialization 'sol::stack::unqualified_pusher<float Vector::*>::push<float Vector::*>' requested here
  878 |                                 return p.push(L, std::forward<T>(t), std::forward<Args>(args)...);
      |                                          ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/stack_field.hpp:222:7: note: in instantiation of function template specialization 'sol::stack::push<float Vector::*>' requested here
  222 |                                                 push(L, std::forward<Value>(value));
      |                                                 ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/stack_core.hpp:1250:59: note: in instantiation of function template specialization 'sol::stack::field_setter<const char *>::set<const char *, float Vector::*>' requested here
 1250 |                         field_setter<meta::unqualified_t<Key>, global, raw> {}.set(L, std::forward<Key>(key), std::forward<Value>(value), tableindex);
      |                                                                                ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/table_core.hpp:264:14: note: (skipping 3 contexts in backtrace; use -ftemplate-backtrace-limit=0 to see all)
  264 |                                                 stack::set_field<global, raw>(L, std::forward<Key>(key), std::forward<Keys>(keys)..., table_index);
      |                                                        ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/usertype.hpp:100:20: note: in instantiation of function template specialization 'sol::basic_table_core<false, sol::basic_reference<false>>::set<const char *, float Vector::*>' requested here
  100 |                                         table_base_t::set(std::forward<Key>(key), std::forward<Value>(value));
      |                                                       ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/usertype_proxy.hpp:64:9: note: in instantiation of function template specialization 'sol::basic_usertype<Vector, sol::basic_reference<false>>::set<const char *, float Vector::*>' requested here
   64 |                                 tbl.set(std::get<I>(std::move(key))..., std::forward<T>(value));
      |                                     ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/usertype_proxy.hpp:86:21: note: in instantiation of function template specialization 'sol::usertype_proxy<sol::basic_usertype<Vector, sol::basic_reference<false>> &, const char *>::tuple_set<0UL, float Vector::*>' requested here
   86 |                         std::move(*this).tuple_set(idx_seq(), std::forward<T>(item));
      |                                          ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/usertype_proxy.hpp:97:28: note: in instantiation of function template specialization 'sol::usertype_proxy<sol::basic_usertype<Vector, sol::basic_reference<false>> &, const char *>::set<float Vector::*>' requested here
   97 |                         return std::move(*this).set(std::forward<T>(other));
      |                                                 ^
/home/runner/work/RosaServer/RosaServer/RosaServer/rosaserver.cpp:87:13: note: in instantiation of function template specialization 'sol::usertype_proxy<sol::basic_usertype<Vector, sol::basic_reference<false>> &, const char *>::operator=<float Vector::*>' requested here
   87 |                 meta["x"] = &Vector::x;
      |                           ^
/home/runner/work/RosaServer/RosaServer/sol2/include/sol/function_types_stateless.hpp:323:14: note: candidate template ignored: substitution failure [with is_yielding = false, no_trampoline = false]
  323 |                 static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
      |                            ^
Razakhel commented 8 months ago

I have this issue too. For what it's worth, replacing upvalue_this_member_variable::call()'s noexcept(std::is_nothrow_copy_assignable_v<T>) specification by either noexcept(true), noexcept(false) or even removing it entirely seems to "fix" it for some reason. I don't really understand that behavior right now, but maybe that can help a bit.

JWatersMeet commented 7 months ago

Same issue here while building MAME, latest MSYS2 has just deployed LLVM 18.1 components.

Possibly related to the following Clang bugfix "Clang will correctly evaluate noexcept expression for template functions of template classes." - see https://prereleases.llvm.org/18.1.0/rc3/tools/clang/docs/ReleaseNotes.html

The workaround from Razakhel above resolves compilation but the binary is non-working.

JWatersMeet commented 7 months ago

Binary is working now with compilation workaround, was unrelated error my part.

cuavas commented 6 months ago

I think this is a bug in clang. The noexcept specification isn’t part of the function type. You can see it accepts it without a noexcept specification at all or with a fixed noexcept (which will obviously cause issues if the assignment can throw an exception). Is it possible to create a minimal case exposing this to report upstream?

cuavas commented 6 months ago

OK, it’s actually pretty easy to reproduce – this builds with clang 17 but not with clang 18 as C++17:

template <typename T>
struct class_tmpl {
    template <bool B> static void call_e() { }
    template <bool B> static void call_ne() noexcept { }
    template <bool B> static void call() noexcept(sizeof(T) > 1) { }
};

int main(int argc, char *argv[])
{
    using function_ptr = void (*)();
    function_ptr f1 = &class_tmpl<int>::call_e<false>;
    function_ptr f2 = &class_tmpl<int>::call_ne<false>;
    function_ptr f3 = &class_tmpl<int>::call<false>;
    return 0;
}

Results:

test.cpp:13:24: error: address of overloaded function 'call' does not match required type 'void ()'
   13 |     function_ptr f3 = &class_tmpl<int>::call<false>;
      |                        ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
test.cpp:5:35: note: candidate template ignored: substitution failure [with B = false]
    5 |     template <bool B> static void call() noexcept(sizeof(T) > 1) { }
      |                                   ^
1 error generated.
cuavas commented 6 months ago

Reported as llvm/llvm-project#91362

cuavas commented 5 months ago

Fixed in llvm main branch by llvm/llvm-project#90760. I don't know when this will make it into a clang release.

checkraisefold commented 5 months ago

Cool, I'll close this as soon as it's in a Clang release and tested

cuavas commented 5 months ago

You can use this workaround in the mean time:

diff --git a/3rdparty/sol2/sol/sol.hpp b/3rdparty/sol2/sol/sol.hpp index 8b0b7d36ea4ef..1a9375d996d2f 100644
--- a/3rdparty/sol2/sol/sol.hpp
+++ b/3rdparty/sol2/sol/sol.hpp
@@ -19416,7 +19416,13 @@ namespace sol { namespace function_detail {
        }

        template <bool is_yielding, bool no_trampoline>
-       static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
+       static int call(lua_State* L)
+#if SOL_IS_ON(SOL_COMPILER_CLANG)
+       // apparent regression in clang 18 - llvm/llvm-project#91362
+#else
+           noexcept(std::is_nothrow_copy_assignable_v<T>)
+#endif
+       {
            int nr;
            if constexpr (no_trampoline) {
                nr = real_call(L);
@@ -19456,7 +19462,13 @@ namespace sol { namespace function_detail {
        }

        template <bool is_yielding, bool no_trampoline>
-       static int call(lua_State* L) noexcept(std::is_nothrow_copy_assignable_v<T>) {
+       static int call(lua_State* L)
+#if SOL_IS_ON(SOL_COMPILER_CLANG)
+       // apparent regression in clang 18 - llvm/llvm-project#91362
+#else
+           noexcept(std::is_nothrow_copy_assignable_v<T>)
+#endif
+       {
            int nr;
            if constexpr (no_trampoline) {
                nr = real_call(L);

That's on the single-header version, but you should be able to work out how to apply it to the source headers.

SiarheiFedartsou commented 5 months ago

Hey @ThePhD What do you think if I make a PR to include workaround from this comment above to main branch? It seems clang-16/clang-17/clang-18 all have this issue.

triplejam commented 3 months ago

I find it ironic that a popular library by someone on the committee is not considered worthy enough of llvm devs to back-port fixes for lol.

TheSevenSages commented 1 month ago

I have this issue too. For what it's worth, replacing upvalue_this_member_variable::call()'s noexcept(std::is_nothrow_copy_assignable_v<T>) specification by either noexcept(true), noexcept(false) or even removing it entirely seems to "fix" it for some reason. I don't really understand that behavior right now, but maybe that can help a bit.

Attempted this fix, did not work for me