-
Notifications
You must be signed in to change notification settings - Fork 312
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
Convert config fields header, trailer, after_includes to text fields. #977
base: master
Are you sure you want to change the base?
Conversation
#if 0 | ||
''' ' | ||
#endif | ||
header = [ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem particularly an improvement fwiw. But after this patch, is there any test that remains with header = "string"
? Does that still work with this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, you can still use strings.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Searching the codebase I found 1 test (swift_name) with header = "string"
and 13 tests (asserted_cast/cfg_2/cfg/custom_header/deprecated/destructor_and_copy_ctor/layout_aligned_opaque/layout_packed_opaque/layout/must_use/nonnull_attribute/mod_attr/rename_crate) with
header = """
multi-line string
"""
Hmm... |
I guess I should give a bit of background into how I arrived here. While waiting for progress in #962 (volatile type qualifier) I attempted to fix other issues I have with cbindgen, like #43 (cyclical pointer references) and others. Random example from pin.toml: header = """
#if 0
''' '
#endif
#ifdef __cplusplus
template <typename T>
using Pin = T;
template <typename T>
using Box = T*;
#endif
#if 0
' '''
#endif
"""
Basically, in this case the contents of header are supposed to affect only the C++ runs of the test, but I was forced to learn how C and C++ and cython treats this text to understand what was really going on. Ideally I would create an issue to get a discussing going so a solution could be ironed out, but it takes sooooo long to get any feedback in this repo and the code does not point to a particular structure or style to emulate, that I decided to just make a PR that can be tested and change whatever is requested. A bunch of solutions were considered:
Cargo.toml allows the version of dependencies to be strings or tables, so I tried to replicate that kind of strategy with 4). I'm not happy with the rust names so any feedback is appreciated. |
Motivation:
Some cbindgen tests have a toml config file.
That config file must work with multiple languages.
Hard to understand tricks are used, making it harder for cbindgen contributors to make tests for their code.
A better solution is to allow language-specific text in text fields that allow "code".
Changes:
This PR attempts to do that, while maintaining backwards compatibility.
I chose a serde-compatible way to represent the data (untagged enums) but can change to whatever you want.
Only header, trailer, after_includes were changed.
Other fields that output raw "code" can be changed too.
No idea how docs.md should be changed so I left it alone.
Fields:
Text field can be a line field or a sequence of lines or structs:
Line field can be a single line or a struct: