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.12k stars 500 forks source link

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

Closed cuavas closed 1 year ago

cuavas commented 1 year ago

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))
Clemapfel commented 1 year ago

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

ThePhD commented 1 year ago

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 commented 1 year ago

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.