-
Notifications
You must be signed in to change notification settings - Fork 378
Use C++ fixed-type enumeration syntax under C23 (or higher) as well #1156
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
Changes from 3 commits
913df52
172da53
8b8a203
6d6f2ad
abaef72
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -680,15 +680,20 @@ impl Enum { | |
| } | ||
| write!(out, " {tag_name}"); | ||
|
|
||
| if config.cpp_compatible_c() { | ||
| out.new_line(); | ||
| out.write("#ifdef __cplusplus"); | ||
| out.new_line(); | ||
| write!(out, " : {prim}"); | ||
| out.new_line(); | ||
| out.write("#endif // __cplusplus"); | ||
| out.new_line(); | ||
| } | ||
| // Write typed enum syntax (valid in C23 or later or C++) | ||
| let cond = if config.cpp_compatible_c() { | ||
| "defined(__cplusplus) || __STDC_VERSION__ >= 202311L" | ||
| } else { | ||
| "__STDC_VERSION__ >= 202311L" | ||
| }; | ||
|
|
||
| out.new_line(); | ||
| write!(out, "#if {cond}"); | ||
| out.new_line(); | ||
| write!(out, " : {prim}"); | ||
| out.new_line(); | ||
| write!(out, "#endif // {cond}"); | ||
| out.new_line(); | ||
|
Comment on lines
+691
to
+696
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Unfortunately, unconditionally outputting this causes issues with some tools that parse header files. In particular, Python's If you try to use them, you get this error: >>> import cffi
>>> ffi = cffi.FFI()
>>> ffi.cdef("#if FOO >= 100\n#endif\n")
cffi.CDefError: cannot parse "#if FOO >= 100"
<cdef source string>:1:1: Directives not supported yetcyphar/libpathrs#382 is an example of how this can cause build failures in a downstream project -- our build scripts strip out any non- But the core issue is the same. It would be nice if this output could be made configurable so that we can continue to use
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would be ok with reviewing a patch to make this opt-in / opt-out. Though, have you considered
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's not a bad idea (and I must admit I hadn't considered it) -- unfortunately shelling out to It would be easier to implement that in Python, at which point it might actually end up being less effort to just add support for basic
What would you think about a patch which let you specify a C version to generate (to not make this switch too special-purpose) and so if you pick C99 mode (for instance) you don't get these Or would you prefer something more like an option for "limited C" and "full-on C"? (That would make things easier for me in the future, I must admit! 😅)
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think something like It'd be a breaking change but it'd remove the ifdefs altogether from the generated code? I guess the ifdef route was taken here mostly because we already had a |
||
| } else { | ||
| if config.style.generate_typedef() { | ||
| out.write("typedef "); | ||
|
|
@@ -759,17 +764,39 @@ impl Enum { | |
| } | ||
|
|
||
| // Emit typedef specifying the tag enum's size if necessary. | ||
| // In C++ enums can "inherit" from numeric types (`enum E: uint8_t { ... }`), | ||
| // but in C `typedef uint8_t E` is the only way to give a fixed size to `E`. | ||
| // In C++ or C23 enums can "inherit" from numeric types (`enum E: uint8_t { ... }`), | ||
| // but in older versions of C `typedef uint8_t E` is the only way to give a fixed size to `E`. | ||
| if let Some(prim) = size { | ||
| if config.cpp_compatible_c() { | ||
| out.new_line_if_not_start(); | ||
| out.write("#ifndef __cplusplus"); | ||
| } | ||
|
|
||
| if config.language != Language::Cxx { | ||
| if config.language == Language::C { | ||
| out.new_line(); | ||
| out.write("#if __STDC_VERSION__ >= 202311L"); | ||
|
|
||
| out.new_line(); | ||
| write!( | ||
| out, | ||
| "{} enum {} {};", | ||
| config.language.typedef(), | ||
| tag_name, | ||
| tag_name | ||
| ); | ||
|
|
||
| out.new_line(); | ||
| out.write("#else"); | ||
| } | ||
|
|
||
| out.new_line(); | ||
| write!(out, "{} {} {};", config.language.typedef(), prim, tag_name); | ||
|
|
||
| if config.language == Language::C { | ||
| out.new_line(); | ||
| out.write("#endif // __STDC_VERSION__ >= 202311L"); | ||
| } | ||
|
Comment on lines
+776
to
+799
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And this change has a very similar issue -- I believe this will cause multiple-defines errors if our build script strips out the |
||
| } | ||
|
|
||
| if config.cpp_compatible_c() { | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.