Re: [Tarantool-patches] [PATCH luajit] Fix C file generation in jit.bcsave.

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hello, Sergey!
Thanks for the fixes!
See my comment below.

On 31.10.23, Sergey Bronnikov wrote:
> Hello,
> 
> On 10/23/23 14:12, Sergey Kaplun wrote:
> > Hi, Sergey!
> > Thanks for the patch!
> > Please consider my comments below.
> >
> > On 19.10.23, Sergey Bronnikov wrote:
> >> From: Sergey Bronnikov <sergeyb@tarantool.org>
> >>
> >> Thanks to codicodi.
> >>
> >> (cherry picked from commit 62903bacf4abbb1c6df0f395553eeaf1f68352c6)
> >>

<snipped>

> >
> > Also, may you please add a comment why `LUA_PATH` is better here instead
> > `LUA_TEST_ENV` (we need only to run the corresponding `jit.` modules)?
> >
> > I suppose that this test is passes even before the patch because its
> > required Lua library instead compiled C library. Maybe it is better to
> > use different names for them to avoid collisions?
> 
> This is a good observation, thanks!
> 
> We cannot use different for exported C file and Lua module,
> 
> because file name is used as a symbol in shared library:
> 
> 
> $ nm 
> build/test/tarantool-tests/lj-551-bytecode-c-broken-macro/bcsaved_clib.so
> 
> <snipped>
> 
> 0000000000002000 R luaJIT_BC_bcsaved_clib
> 
> <snipped>
> 
> 
> I did it in the following way: Lua module in source dir has name 
> bcsaved.lua, then we copying this file to
> 
> a current binary dir with another name bcsaved_clib.lua, then we export 
> bc code to C and *remove* Lua module
> 
> in a current binary dir:
> 
> 
> diff --git 
> a/test/tarantool-tests/lj-551-bytecode-c-broken-macro/CMakeLists.txt 
> b/test/tarantool-tests/lj-551-bytecode-c-broken-macro/CMakeLists.txt
> index cd685b5b..45857386 100644
> --- a/test/tarantool-tests/lj-551-bytecode-c-broken-macro/CMakeLists.txt
> +++ b/test/tarantool-tests/lj-551-bytecode-c-broken-macro/CMakeLists.txt
> @@ -1,5 +1,6 @@
>   set(LIB_NAME "bcsaved_clib")
> -set(LUA_FILE ${CMAKE_CURRENT_SOURCE_DIR}/${LIB_NAME}.lua)
> +set(LUA_FILE_ORIG ${CMAKE_CURRENT_SOURCE_DIR}/bcsaved.lua)
> +set(LUA_FILE ${CMAKE_CURRENT_BINARY_DIR}/${LIB_NAME}.lua)
>   set(C_FILE ${LIB_NAME}.c)
> 
>   make_lua_path(LUA_PATH
> @@ -8,11 +9,24 @@ make_lua_path(LUA_PATH
>       ${PROJECT_SOURCE_DIR}/src/jit/?.lua
>   )
> 
> +add_custom_target(copy_lua
> +  COMMAND ${CMAKE_COMMAND} -E
> +    copy
> +      ${LUA_FILE_ORIG}
> +      ${LUA_FILE}
> +  DEPENDS ${LUA_FILE_ORIG}
> +  BYPRODUCTS ${LUA_FILE}
> +  COMMENT "Copying Lua module"
> +  VERBATIM
> +)
> +
>   add_custom_target(export_bc
>     COMMAND ${CMAKE_COMMAND} -E
>       env
>         LUA_PATH=${LUA_PATH}
>       ${LUAJIT_BINARY} -b ${LUA_FILE} ${C_FILE}
> +  COMMAND ${CMAKE_COMMAND} -E
> +    remove ${LUA_FILE}
>     DEPENDS luajit-main ${LUA_FILE}
>     BYPRODUCTS ${C_FILE}
>     COMMENT "Exporting bytecode to a C file"

The test still doesn't fail before the commit.
The reason is that the C file is compiled by C compiler, not the C++
compiler (so this macro is irrelevant).

I suppose that we should add a .cpp file for the test too (and add the
LINKER_LANGUAGE CXX [1] property for this target if its necessary).

[1]: https://cmake.org/cmake/help/latest/prop_tgt/LINKER_LANGUAGE.html#linker-language

-- 
Best regards,
Sergey Kaplun