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

WIP: move abi checks to a feature and build the C code at build time #1052

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

pbor
Copy link
Contributor

@pbor pbor commented Feb 7, 2021

The abi tests are now conditional to the abi-tests feature,
when that is enabled, the C code for the abi tests is now built
at compile time, using the cc crate rather than having our own
Compiler abstraction and having to compile stuff at runtime and
deal with temp directories etc.

src/codegen/sys/build.rs Outdated Show resolved Hide resolved
@sdroege
Copy link
Member

sdroege commented Feb 7, 2021

Seems like a good idea

@pbor pbor force-pushed the c_abi branch 3 times, most recently from cead587 to 16d60f7 Compare February 7, 2021 17:54
@pbor
Copy link
Contributor Author

pbor commented Feb 7, 2021

This currently fails for gdk-pixbuf due to gdesmott/system-deps#28

With regard to Windows CI, the good news is that this approach works and the C programs are compiled and the layout test work, the bad news is the MSVC does not support _Generic so the current code in constant.c does not work... I am not sure if we have a better idea about that (use C++?) or simply skip that test on windows

@sdroege
Copy link
Member

sdroege commented Feb 8, 2021

It's available since Visual Studio 2019 version 16.7.0 from what I can see, and you can enable it via /std:c11. Should maybe just require that version for running the tests? What version is available with GitHub actions?

@pbor
Copy link
Contributor Author

pbor commented Feb 9, 2021

It's available since Visual Studio 2019 version 16.7.0 from what I can see, and you can enable it via /std:c11. Should maybe just require that version for running the tests? What version is available with GitHub actions?

Actually it seems that the Visual Studio version is new enough and does have _Generic, but however it does not like how we use it:

type 'char' in _Generic association compatible with previous association type 'char'

@sdroege
Copy link
Member

sdroege commented Feb 9, 2021

That means we have char in there twice, which we probably shouldn't and it's gcc's fault to not complain about that

@pbor
Copy link
Contributor Author

pbor commented Feb 9, 2021

the problem is that we have:

        char: "%c", \
        signed char: "%hhd", \
        unsigned char: "%hhu", \

so we are printing them differently

@sdroege
Copy link
Member

sdroege commented Feb 9, 2021

Ah that's annoying and otherwise needs conditional compilation for platforms where signed or unsigned chars are the default...

@sdroege
Copy link
Member

sdroege commented Feb 9, 2021

OTOH, only the two cases (signed / unsigned) should be there. char alone is not really needed

@pbor
Copy link
Contributor Author

pbor commented Feb 9, 2021

That has helped, but now it complains about not finding <stdalign.h>, which as far as I understand should be there in MSVC when using c11. Meh.

The abi tests are now conditional to the abi-tests feature,
when that is enabled, the C code for the abi tests is now built
at compile time, using the cc crate rather than having our own
Compiler abstraction and having to compile stuff at runtime and
deal with temp directories etc.
We explicitely handle signed and unsigned, and some compilers (MSVC)
do not like duplicate types.
@pbor pbor force-pushed the c_abi branch 2 times, most recently from fc9de62 to 7b0ff6d Compare February 10, 2021 13:30
@jf2048 jf2048 mentioned this pull request Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants