luau icon indicating copy to clipboard operation
luau copied to clipboard

Prevent `extern "C"` propagation

Open Jan200101 opened this issue 2 years ago • 8 comments

extern "C" is C++ specific and if we enable it our consumers are likely not C++.

Jan200101 avatar May 19 '23 18:05 Jan200101

Sorry for the long response, we had to find someone who knows how CMake works.

This change will result in anything downstream from Luau.CodeGen that relies on LUACODEGEN_API having to also define it themselves, if they don't want to get linker errors. If someone is linking against Luau.CodeGen from CMake and they're not C++, they do get the problem you are trying to solve, but this use case is rare - for a C library in CMake to link against a C++ library.

The better solution would have been to use a generator expression like $<$<COMPILE_LANGUAGE:CXX>:LUACODE_API=extern\"C\"), but that requires CMake 3.15+. I believe we can do the same using an additional if statement to check for COMPILE_LANGUAGE.

vegorov-rbx avatar Jun 02 '23 13:06 vegorov-rbx

The better solution would have been to use a generator expression like $<$<COMPILE_LANGUAGE:CXX>:LUACODE_API=extern\"C\"), but that requires CMake 3.15+. I believe we can do the same using an additional if statement to check for COMPILE_LANGUAGE.

The generator expressions for the language appears to have been introduced with cmake 3.3 not 3.15, but that is still above the minimum version luau requires.

I don't think a single if would do the job, the if statement would be evaluated while luau is being configured, at which point CXX would be part of COMPILE_LANGUAGE.

I'm still looking around for other alternatives, but a massive workaround would be setting CMAKE_CXX_FLAGS

Jan200101 avatar Jun 04 '23 13:06 Jan200101

This change breaks exporting to a static and dynamic library which breaks embedding Luau in e.g CSharp. This is not a useful change and there should be specific settings added for exporting to a DLL, static library and basic extern "C" rather than one single setting

@Jan200101 @vegorov-rbx

This change breaks exporting to a static and dynamic library which breaks embedding Luau in e.g CSharp.

As far as I can tell the cmake setup does not currently support building a dyanamic library, so this appears to a moot point, though it does affect how Luau would be consumed by C++ projects, breaking compatibility there.

This is not a useful change and there should be specific settings added for exporting to a DLL, static library and basic extern "C" rather than one single setting

This change needs to be done somehow regardless.

Most languages rely on the C ABI for their FFI and those that can consume the header files (e.g. C, Zig, etc.) may not understand extern "C".

An alternative (that I think would be best overall) would be dropping the build option and forcing extern C on the entire header when C++ is used.

Jan200101 avatar Jun 09 '23 13:06 Jan200101

@zeux I wonder if we should always use "C" linkage without relying on CMake and place them in a block like:

#ifndef __cplusplus
extern "C" {
#endif

vegorov-rbx avatar Jun 15 '23 14:06 vegorov-rbx

Unfortunately we can't use extern "C" unconditionally when LUA_USE_LONGJMP is set to 0 -- this breaks exception handling because MSVC doesn't generate unwind tables, so calling functions like luaL_checknumber will abort instead of throwing a catchable exception.

It does seem like we'd need to have a separate LUA_EXTERN_C define :-/ I think the DLL export/import can still be handled as we do now (with CMake configuration without explicit support)?

zeux avatar Jun 20 '23 16:06 zeux

I ran into this when trying to use Luau from a project written in C. When LUAU_EXTERN_C is enabled, C files can't include Luau headers, and when it's off C projects can't link to Luau libraries since they don't have C symbols.

In case it helps anyone, I have the following workaround which manually rips out the extern "C" compile definitions for projects linking to luau targets:

foreach(target Luau.VM Luau.Compiler)
  get_target_property(defs ${target} INTERFACE_COMPILE_DEFINITIONS)
  list(FILTER defs EXCLUDE REGEX [[LUA.*_API]])
  set_target_properties(${target} PROPERTIES INTERFACE_COMPILE_DEFINITIONS "${defs}")
endforeach()

# target_link_libraries(myproject Luau.VM Luau.Compiler Luau.Ast) etc.

This allows me to use luau as-is without having to apply this change on a fork.

bjornbytes avatar Jun 06 '24 16:06 bjornbytes

I ran into this when trying to use Luau from a project written in C. When LUAU_EXTERN_C is enabled, C files can't include Luau headers, and when it's off C projects can't link to Luau libraries since they don't have C symbols.

In case it helps anyone, I have the following workaround which manually rips out the extern "C" compile definitions for projects linking to luau targets:

foreach(target Luau.VM Luau.Compiler)
  get_target_property(defs ${target} INTERFACE_COMPILE_DEFINITIONS)
  list(FILTER defs EXCLUDE REGEX [[LUA.*_API]])
  set_target_properties(${target} PROPERTIES INTERFACE_COMPILE_DEFINITIONS "${defs}")
endforeach()

# target_link_libraries(myproject Luau.VM Luau.Compiler Luau.Ast) etc.

This allows me to use luau as-is without having to apply this change on a fork.

This snippet fixed my issue just now when trying to do the exact same thing; Luau in a C project. Thanks! Should be corrected; Luau can't be embedded in C projects easily at all with this caveat.

checkraisefold avatar Jun 14 '24 11:06 checkraisefold