conditional fields of constexpr literal structs - #989
#989Merged
emilio merged 2 commits intoMay 20, 2025
mozilla:mastermozilla/cbindgen:masterfrom
youknowone:constexpr-conditional-fieldyouknowone/cbindgen:constexpr-conditional-fieldCopy head branch name to clipboard
Merged
conditional fields of constexpr literal structs#989emilio merged 2 commits intomozilla:mastermozilla/cbindgen:masterfrom youknowone:constexpr-conditional-fieldyouknowone/cbindgen:constexpr-conditional-fieldCopy head branch name to clipboard
emilio merged 2 commits into
mozilla:mastermozilla/cbindgen:masterfrom
youknowone:constexpr-conditional-fieldyouknowone/cbindgen:constexpr-conditional-fieldCopy head branch name to clipboard
Conversation
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
August 12, 2024 12:47
b890eef to
ddd17f7
Compare
emilio
approved these changes
Oct 27, 2024
emilio
left a comment
Collaborator
There was a problem hiding this comment.
Let's do this, just some nits about whitespace, but looks good.
| write!(out, "/* .{} = */ ", ordered_key); | ||
| self.write_literal(out, &lit.value); | ||
| if i + 1 != ordered_fields.len() { | ||
| write!(out, ", "); |
Collaborator
There was a problem hiding this comment.
This adds trailing white-space, please remove the space if is_constexpr?
| write!(out, "{}", export_name); | ||
| } | ||
|
|
||
| write!(out, "{{ "); |
Collaborator
There was a problem hiding this comment.
Same, this space needs to be removed.
Collaborator
|
Sorry for the lag getting to this btw :( |
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
October 28, 2024 06:57
ddd17f7 to
df9f46e
Compare
Contributor
Author
|
@emilio Thank you for the review! I updated the both PRs |
emilio
force-pushed
the
constexpr-conditional-field
branch
from
October 28, 2024 15:09
df9f46e to
62c856f
Compare
Collaborator
|
Could you rebase this please? Thanks! |
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
October 29, 2024 07:55
62c856f to
cea2ef2
Compare
Contributor
Author
|
rebased |
Contributor
Author
|
@emilio Could you check it again? |
youknowone
marked this pull request as draft
November 4, 2024 04:21
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
November 4, 2024 04:45
cea2ef2 to
2ed778c
Compare
youknowone
marked this pull request as ready for review
November 4, 2024 04:45
Contributor
Author
|
I updated my local patch to this branch. It worked well for the project. |
Contributor
Author
|
@emilio is there any blocker? |
Contributor
Author
|
@emilio reminding |
youknowone
force-pushed
the
constexpr-conditional-field
branch
from
March 26, 2025 02:15
2ed778c to
64c80d2
Compare
Contributor
Author
|
rebased |
Contributor
Author
|
@emilio reminding when you come back |
emilio
approved these changes
May 20, 2025
Collaborator
|
Thaks for the ping! |
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.
Partially fix #955 about constexpr
#988 is included in this PR
Because constexpr and defines require very different approach about this problem, I created another PR for constexpr fix while keeping the other one for define fix.