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

Doesn't build with Clang 18.1.0 #1581

Open
checkraisefold opened this issue Feb 16, 2024 · 12 comments
Open

Doesn't build with Clang 18.1.0 #1581

checkraisefold opened this issue Feb 16, 2024 · 12 comments

Comments

@checkraisefold
Copy link

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
Copy link

Razakhel commented Mar 3, 2024

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
Copy link

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
Copy link

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

@cuavas
Copy link
Contributor

cuavas commented May 7, 2024

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
Copy link
Contributor

cuavas commented May 7, 2024

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
Copy link
Contributor

cuavas commented May 7, 2024

Reported as llvm/llvm-project#91362

@cuavas
Copy link
Contributor

cuavas commented May 9, 2024

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

@checkraisefold
Copy link
Author

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

@cuavas
Copy link
Contributor

cuavas commented May 9, 2024

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
Copy link

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
Copy link

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.

lyrahgames added a commit to build2-packaging/sol2 that referenced this issue Jul 30, 2024
- Update `README.md`
- Apply workaround for bug in Clang 18
  See: ThePhD/sol2#1581
- Use symlinks for all library source files
  This allows to apply custom workarounds for external bugs.
- Run tests as independent package
  Currently, the latest `catch2` package from `cppget.org` fails to
  compile with Clang and leads to false-negative results.
- Add `sol2-examples` package with upstream examples
- Add `sol2-tests` package with upstream tests
- Update `manifest`
- Tweak and clean up `buildfile`s
- Add `README.md` and `PACKAGE-README.md` symlinks
- Add symlinks to Sol2 license file
- Transform into multi-package repository
- Add `.gitattributes`
- Update `.gitignore`
- Use standard build2 naming scheme
  None of the standard build2 filenames clash with upstream source code.
  Revert alternative naming scheme to be canonical.

Issue: #1
ndoxx added a commit to ndoxx/sol2 that referenced this issue Aug 6, 2024
scarf005 added a commit to scarf005/Cataclysm-BN that referenced this issue Sep 27, 2024
it's a known llvm bug that isn't backported to clang 18 yet. applied workaround to sol2 instead.

see: ThePhD/sol2#1581 (comment)
chaosvolt pushed a commit to cataclysmbnteam/Cataclysm-BN that referenced this issue Sep 27, 2024
it's a known llvm bug that isn't backported to clang 18 yet. applied workaround to sol2 instead.

see: ThePhD/sol2#1581 (comment)
@TheSevenSages
Copy link

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants