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

Not possible to build sol2 v3.3.0 with Lua 5.3 with bitlib compatibility enabled #1461

Closed
cuavas opened this issue Mar 6, 2023 · 3 comments

Comments

@cuavas
Copy link
Contributor

cuavas commented Mar 6, 2023

The issue is an apparent misinterpretation of how Lua compatibility macros are defined.

Consider this fragment from Lua’s src/luaconf.h (starts at line 303 in Lua 5.3.4):

#if defined(LUA_COMPAT_5_2)	/* { */

/*
@@ LUA_COMPAT_MATHLIB controls the presence of several deprecated
** functions in the mathematical library.
*/
#define LUA_COMPAT_MATHLIB

/*
@@ LUA_COMPAT_BITLIB controls the presence of library 'bit32'.
*/
#define LUA_COMPAT_BITLIB

/*
@@ LUA_COMPAT_IPAIRS controls the effectiveness of the __ipairs metamethod.
*/
#define LUA_COMPAT_IPAIRS

/*
@@ LUA_COMPAT_APIINTCASTS controls the presence of macros for
** manipulating other integer types (lua_pushunsigned, lua_tounsigned,
** luaL_checkint, luaL_checklong, etc.)
*/
#define LUA_COMPAT_APIINTCASTS

#endif				/* } */

Note that if Lua 5.2 compatibility is enabled, LUA_COMPAT_BITLIB will be defined as an empty macro.

Now consider this code in sol2’s include/sol/compatibility/lua_version.hpp (beginning at line 186 in sol2 v3.3.0):
https://github.com/ThePhD/sol2/blob/v3.3.0/include/sol/compatibility/lua_version.hpp#L186

#if defined (SOL_LUA_BIT32_LIB)
	#if SOL_LUA_BIT32_LIB != 0
		#define SOL_LUA_BIT32_LIB_I_ SOL_ON
	#else
		#define SOL_LUA_BIT32_LIB_I_ SOL_OFF
	#endif
#else
	// Lua 5.2 only (deprecated in 5.3 (503)) (Can be turned on with Compat flags)
	// Lua 5.2, or other versions of Lua with the compat flag, or Lua that is not 5.2 with the specific define (5.4.1 either removed it entirely or broke it)
	#if (SOL_LUA_VERSION_I_ == 502) || (defined(LUA_COMPAT_BITLIB) && (LUA_COMPAT_BITLIB != 0)) || (SOL_LUA_VERSION_I_ < 504 && (defined(LUA_COMPAT_5_2) && (LUA_COMPAT_5_2 != 0)))
		#define SOL_LUA_BIT32_LIB_I_ SOL_ON
	#else
		#define SOL_LUA_BIT32_LIB_I_ SOL_DEFAULT_OFF
	#endif
#endif

On line 195, this fragment causes a compilation error with clang and GCC: (defined(LUA_COMPAT_BITLIB) && (LUA_COMPAT_BITLIB != 0))

Because LUA_COMPAT_BITLIB is defined as an empty macro, it’s substituted as (1 && ( != 0)) so the compilers produce an error because there’s no left-hand argument to the not-equal operator (!=).

The LUA_COMPAT_* macros should only be tested for being defined or undefined. The Lua headers define them as empty macros when enabled, so attempting to compare them to 1/0 causes compilation errors.

I can open a PR to change line 195 to this if it would be considered a good solution:

	#if (SOL_LUA_VERSION_I_ == 502) || defined(LUA_COMPAT_BITLIB) || (SOL_LUA_VERSION_I_ < 504 && defined(LUA_COMPAT_5_2))
cuavas referenced this issue in mamedev/mame Mar 6, 2023
Compile Lua as C++.  When Lua is compiled as C, it uses setjmp/longjmp
for error handling, resulting in failure to unwind intermediate stack
frames.  Trying to ensure no objects with non-trivial destructors are in
scope when raising a Lua error is error-prone.  In particular,
converting an exception to a Lua error becomes convoluted, and raising a
Lua error from a constructor is effectively impossible.

Updated Lua to 5.4.4 - this includes a brand-new garbage collector
implementation with better performance.  The main thing removed is the
deprecated bitlib.

Updated sol2 to version 3.3.0 - this adds support for Lua 5.4 and fixes
a number of issues, including not correctly handling errors when Lua is
built as C++.

Updated LuaFileSystem to version 1.8.0 - this adds support for symbolic
links on Windows, as well as Lua 5.4 compatibility.

Updated LuaSQLite3 to version 0.9.5 - this fixes issues in
multi-threaded environments, as well as Lua 5.4 compatibility.

Fixed double-free after attempting to construct a debugger expression
from Lua with an invalid string, and exposed expression error to Lua in
a better way.

Added warning level print function to Lua.

Fixed saving cheats with shift operators in expressions, although this
code isn't actually used as there's no cheat editor.
@Clemapfel
Copy link

Bump, I can reproduce this and can confirm that the proposed fix solves the issue. It seems to be just a typo to compare against 0 instead of the proper defined

lmoureaux added a commit to lmoureaux/freeciv21 that referenced this issue Apr 5, 2023
lmoureaux added a commit to lmoureaux/sol2 that referenced this issue Apr 5, 2023
Lua may define the macros but leaves them empty. The code assumed they would
contain a boolean value.

This is required to use sol2 with system Lua on Fedora 37.

Closes ThePhD#1461.
jwrober pushed a commit to longturn/freeciv21 that referenced this issue Apr 6, 2023
lmoureaux added a commit to longturn/freeciv21 that referenced this issue Jun 17, 2023
See ThePhD/sol2#1461.
Closes #1895.

(cherry picked from commit 974262c)
@ThePhD
Copy link
Owner

ThePhD commented Jul 18, 2023

This is not a typo, it's intentional. There should be a value in there, and it should be an integer -- any integer at all, indicating if it's on or off.

@cuavas
Copy link
Contributor Author

cuavas commented Jul 18, 2023

This is not a typo, it's intentional. There should be a value in there, and it should be an integer -- any integer at all, indicating if it's on or off.

That’s not correct though. If you look at the Lua source, it does not check for “a value in there”, it simply checks whether the macro is defined or not. #define LUA_COMPAT_BITLIB has the same effect as #define LUA_COMPAT_BITLIB 0 or #define LUA_COMPAT_BITLIB 1. When you define LUA_COMPAT_5_2, Lua itself defines LUA_COMPAT_BITLIB as an empty macro, enabling the feature.

You’re assuming Lua uses the same semantics for its feature macros that you use for yours. This assumption is incorrect.

adityakpandare added a commit to quinoacomputing/cmake-modules that referenced this issue Jul 1, 2024
adityakpandare added a commit to quinoacomputing/quinoa-tpl that referenced this issue Jul 1, 2024
adityakpandare added a commit to quinoacomputing/quinoa that referenced this issue Jul 1, 2024
7f4e7f9b8 fix for sol2 compilation issue ThePhD/sol2#1461

git-subtree-dir: cmake
git-subtree-split: 7f4e7f9b85076bac8823d7cab35153ff2e5de689
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants