search

hpyproject / hpy

HPy: a better API for Python
https://hpyproject.org
MIT License
1.02k stars 52 forks source link

Do not compile helpers with extension by default. #310

Open fangerer opened 2 years ago

fangerer commented 2 years ago

Right now, we are adding source files argparse.c, buildvalue.c, and helpers.c to the source file list of the HPy extension to be built and then they are compiled together with the extension's sources.

We do so because the goal is that the helpers binary code should be in the HPy extension's library. While this works in most cases, we recently discovered a problem with that strategy on Mac (using LLVM toolchain). The extension was written in C++ and added:

extra_compile_args=['-std=c++11', '-stdlib=libc++']
extra_link_args=['-stdlib=libc++']

The extension failed to build argparse.c with following error:

gcc -Wno-unused-result -Wsign-compare -g -O0 -Wall -DHPY -DHPY_UNIVERSAL_ABI -I. -I/path/to/lib/python3.8/site-packages/hpy/devel/include -I/path/to/include -I/Users/fa/work/repos/Python-3.8.5/Include -I/Users/fa/work/repos/Python-3.8.5 -c /path/to/lib/python3.8/site-packages/hpy/devel/src/runtime/argparse.c -o build/temp.macosx-12.0-x86_64-3.8-pydebug/path/to/lib/python3.8/site-packages/hpy/devel/src/runtime/argparse.o -std=c++11 -stdlib=libc++
error: invalid argument '-std=c++11' not allowed with 'C'
error: command 'gcc' failed with exit status 1

IMO, the underlying issue is that we are building these sources with the flags of the target. I think we should build them in the same way as we do with the rest of HPy. So, I suggest that we build an archive for the helpers and statically link them into the extension.

Optionally, if someone really wants to compile the helpers with the extension, we can provide an option to do so.

hodgestar commented 2 years ago

Would adding

    #ifdef __cplusplus
    extern "C" {
    #endif

blocks to the relevant .c files help at all?

fangerer commented 2 years ago

Unfortunately, this does not help (just tried it out).

hodgestar commented 2 years ago

Unfortunately, this does not help (just tried it out).

Pity! Thanks for trying it out.

steve-s commented 2 years ago

We should perhaps remove the extern "C" blocks in ctx_*.c files. I searched around and afaics it just disables names mangling, but the C++ semantics stays the same. I'd say that compiling those files with C++ compiler is just not supported. We should try to keep the header files, especially the inline functions, in some common subset of C and C++ if possible though.