Skip to content

translate_c: fix ternary operator output in C macros#24664

Closed
cmpute wants to merge 1 commit into
ziglang:masterfrom
cmpute:fix-ternary-in-macro
Closed

translate_c: fix ternary operator output in C macros#24664
cmpute wants to merge 1 commit into
ziglang:masterfrom
cmpute:fix-ternary-in-macro

Conversation

@cmpute

@cmpute cmpute commented Aug 2, 2025

Copy link
Copy Markdown

this is a fix for ziglang/translate-c#44 . Basically it tries to wrap all the conditions in ternary operation with a zero check.

@Vexu

Vexu commented Aug 2, 2025

Copy link
Copy Markdown
Member

This version of translate-c is going to be deleted in favor of ziglang/translate-c: #24497. You should contribute this there instead.

@cmpute

cmpute commented Aug 3, 2025

Copy link
Copy Markdown
Author

This version of translate-c is going to be deleted in favor of ziglang/translate-c: #24497. You should contribute this there instead.

Per this comment: #24546 (comment), maybe let's see if this can land in 0.15?

@ehaas

ehaas commented Aug 3, 2025

Copy link
Copy Markdown
Contributor

Will this work if the check function returns a bool instead of int? I think full-featured support for the ternary operator in macros will require a helper function because any scalar type (ints, bools, floats, enums, pointers) can be used in the condition and the two result types are converted to a common type if necessary.

@cmpute

cmpute commented Aug 4, 2025

Copy link
Copy Markdown
Author

Will this work if the check function returns a bool instead of int? I think full-featured support for the ternary operator in macros will require a helper function because any scalar type (ints, bools, floats, enums, pointers) can be used in the condition and the two result types are converted to a common type if necessary.

This doesn't support other scalar types yet. It's also necessary to improve the ternary operator translation in normal (non-macro) code, which currently also uses the macroIntToBool function.

@cmpute

cmpute commented Aug 22, 2025

Copy link
Copy Markdown
Author

Any possibilities for this to be merged by the next minor version?

@Vexu Vexu added this to the 0.15.2 milestone Aug 22, 2025
@alexrp

alexrp commented Sep 16, 2025

Copy link
Copy Markdown
Member

@Vexu if this is indeed intended for 0.15.2, can you review it?

@Vexu

Vexu commented Sep 16, 2025

Copy link
Copy Markdown
Member

The fix looks good and should be non-breaking but since my priority is ziglang/translate-c I don't care too much about it making it to 0.15.2. The issue should remain open/be transferred if this is merged since this is also broken in the new implementation.

@alexrp

alexrp commented Sep 29, 2025

Copy link
Copy Markdown
Member

@cmpute can you please retarget the PR against the 0.15.x branch? (Might need to reopen it to do so.)

@cmpute

cmpute commented Sep 30, 2025

Copy link
Copy Markdown
Author

@alexrp Could you please provide some guidance on how to add translate-c test in the latest master? I can't find where are those translate-c test cases

@alexrp

alexrp commented Sep 30, 2025

Copy link
Copy Markdown
Member

master uses https://github.com/ziglang/translate-c

@cmpute

cmpute commented Sep 30, 2025

Copy link
Copy Markdown
Author

@alexrp I got it, I moved this commit to branch 0.15.x, please checkout #25404

@alexrp

alexrp commented Sep 30, 2025

Copy link
Copy Markdown
Member

Thanks, I'll close this then. But feel free to submit a PR to https://github.com/ziglang/translate-c if the bug still exists on master though.

@alexrp alexrp closed this Sep 30, 2025
@alexrp alexrp removed this from the 0.15.2 milestone Oct 1, 2025
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.

4 participants