Repository navigation
Add opt-in C23 constexpr scalar constants - #1190
Draft
GtechGovind wants to merge 2 commits into
Draft
GtechGovind wants to merge 2 commits into
GtechGovind wants to merge 2 commits into
Conversation
emilio
requested changes
Oct 9, 2026
| String::from_utf8(output).unwrap() | ||
| } | ||
|
|
||
| const ENABLED: &str = "[const]\nallow_constexpr_in_c = true\n"; |
Collaborator
There was a problem hiding this comment.
Use the existing test infrastructure please. If we need to update the compiler in CI that's fine.
| out.write(")("); | ||
| // C23 requires exact representability. The explicit conversion | ||
| // performs the narrowing normally applied by initialization, | ||
| // e.g. for f32 literals or integer-promoted bitwise expressions. |
Collaborator
There was a problem hiding this comment.
This comment looks like it should be above the cast. Also could be less verbose:
// C23 doesn't do implicit conversions on constexpr constants.
Or so.
| Literal::BinOp { left, op, right } => { | ||
| // Unsuffixed Rust literals inherit their integer type, whereas | ||
| // C evaluates them as int before our final conversion. Arithmetic | ||
| // and shifts can therefore overflow even for valid Rust constants. |
Collaborator
There was a problem hiding this comment.
I believe this is an issue for C++ as well, remove the matches!() check. The common case is that it won't overflow, and if it does the compiler should warn.
| } | ||
| Literal::PostfixUnaryOp { op, value } => { | ||
| // Negating an inferred C int intermediate can overflow even | ||
| // when the wider Rust destination makes the expression valid. |
Collaborator
There was a problem hiding this comment.
Same, what are these hacks about?
| fn can_be_c_constexpr(&self, allow_float: bool) -> bool { | ||
| match self { | ||
| Literal::Expr(value) => { | ||
| (value.starts_with('\'') && !value.contains("\\u{")) |
Collaborator
There was a problem hiding this comment.
This is a massive hack, I don't think this is acceptable.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I added an opt-in
[const] allow_constexpr_in_csetting for typed primitive C23 constants. It defaults to false, so existing C headers stay unchanged, and it is independent of the C++ settings.I reused the existing constexpr eligibility check and initializer renderer instead of inspecting literal spellings or restricting arithmetic operators. Primitive constants receive an explicit conversion to the declared type; pointers, arrays, aggregates and aliases keep their macro output. The documentation covers the C23 compiler requirement, initializer diagnostics and preprocessor compatibility implications.
The C23 tests now use the existing fixture harness and checked-in expectations across all three C declaration styles, C++ compatibility, C++ and Cython. The fixture covers scalar types, bounds, arithmetic, shifts, compound negation, associated constants and macro fallbacks. The harness selects
-std=c23for fixtures that enable the setting, and CI installs GCC 14 for the compiler checks.Validation:
CBINDGEN_TEST_VERIFY=1 CC=/opt/homebrew/bin/gcc-16 cargo +nightly testwith Cython 3.0.2: 176 passed, zero failed; one existing doc test ignored. All 161 pre-existing fixtures passed unchanged.CC=/usr/bin/clang: passed with Apple Clang 21, including C++ and Cython variants.cargo +stable fmt --check,cargo +stable clippy --workspace -- -D warnings,cargo +stable build --no-default-featuresandgit diff --check: passed.cargo +1.74.0 test nonexistent-test --verbose: passed the repository's MSRV compilation check (no tests selected).cargo +stable package --allow-dirty: packaging and verification build passed. Full nightly tests from the packaged crate: 148 passed, zero failed; one existing doc test ignored.Shared literal lowering is unchanged. Generated initializers still need to be valid C23 constant expressions; this change does not infer Rust intermediate types or evaluate constants. The additional
cargo +stable clippy --all-targets -- -D warningscheck reports the pre-existing eight-argumentrun_cbindgenharness function; the prescribed workspace check passes. Linux/ARM CI has not been reproduced locally.Fixes #1095.