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

C Stringification: prepend __extension__ before the typedef with __int128 #906

Merged
merged 5 commits into from
Feb 4, 2021

Conversation

hannesm
Copy link
Contributor

@hannesm hannesm commented Feb 3, 2021

This allows the generated C code to compile with gcc and -Wpedantic.

Suggested in #820 by @chjj

…t128

This allows the generated C code to compile with gcc and -Wpedantic.

Suggested in mit-plv#820 by @chjj
@JasonGross
Copy link
Collaborator

Thanks! Please also regenerate the C files (run make c-files or just make and let the build complete). Let's also add -Wpedantic to the cflags we test with by adding it at

$(CC) -Wall -Wno-unused-function -Werror $(CFLAGS) -c $(ALL_C_FILES) $(EXTRA_C_FILES)

The current CI error is due to a packaging issue with my ppa which I hope will be fixed within an hour or so (but may take up to a couple of days).

@hannesm
Copy link
Contributor Author

hannesm commented Feb 3, 2021

@JasonGross sure, done in 602069d (feel free to squash if that's preferred)

@JasonGross
Copy link
Collaborator

Thanks! This doesn't seem to work, though:

fiat-c/src/curve25519_64.c:24:31: error: ISO C does not support ‘__int128’ types [-Werror=pedantic]
 FIAT_EXTENSION typedef signed __int128 fiat_25519_int128;

@JasonGross
Copy link
Collaborator

Maybe you meant __GNUC__ rather than GNUC?

@hannesm
Copy link
Contributor Author

hannesm commented Feb 3, 2021

indeed, sorry about that. I did not notice since I'm using on my development machine clang and there no such warning is issued, even with -Wpedantic. I fixed it in f3fc8ce (using __GNUC__).

@chjj
Copy link

chjj commented Feb 3, 2021

It might be wise to have a different prefix for the extension macro in each file: fiat_p224_extension, fiat_p256_extension etc. Otherwise GCC will complain about macro redefinition if two files are included.

src/Stringification/C.v Outdated Show resolved Hide resolved
Co-authored-by: Jason Gross <jasongross9@gmail.com>
@hannesm
Copy link
Contributor Author

hannesm commented Feb 3, 2021

sure, as you like. (running make c-files again and will push an update). I'm not entirely sure which C compilers fiat-crypto targets -- in both clang and gcc we can actually emit __extension__ unconditionally (they also both define __GNUC__)...

@JasonGross JasonGross merged commit e7728d6 into mit-plv:master Feb 4, 2021
@JasonGross
Copy link
Collaborator

Thanks!

@hannesm hannesm deleted the extension branch February 4, 2021 15:13
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.

None yet

3 participants