Skip to content

transpile: Flip comparison operators when negating in CFG#1773

Merged
ahomescu merged 1 commit into
immunant:masterfrom
Rua:negate-comparisons
May 22, 2026
Merged

transpile: Flip comparison operators when negating in CFG#1773
ahomescu merged 1 commit into
immunant:masterfrom
Rua:negate-comparisons

Conversation

@Rua
Copy link
Copy Markdown
Contributor

@Rua Rua commented May 3, 2026

Produces cleaner code, with relatively small changes.

@ahomescu
Copy link
Copy Markdown
Contributor

ahomescu commented May 5, 2026

I think we need to have a discussion before landing this. Two arguments against it:

  • It takes us farther from the original C code, and
  • It looks like something that the refactorer could trivially do.

@Rua
Copy link
Copy Markdown
Contributor Author

Rua commented May 5, 2026

If a small change could mean that the transpiler produces better code without the refactorer, I think that's still a bonus. But your first point is true. I just don't know how faithful the transpiler already tries to be.

In this particular case, the negations already happen because the relooper has done some significant changes to the structure of the code, such as translating do ... while into loop ... break or fixing up goto. So simplifying the break condition seems like a relatively minor change in comparison.

@ahomescu
Copy link
Copy Markdown
Contributor

ahomescu commented May 6, 2026

I flagged this one as well in our internal c2rust discussion. I'm not super in favor, but if the others like it we can land it.

@randomPoison
Copy link
Copy Markdown
Contributor

This seems like a lateral move: The code is a bit simpler and more idiomatic, but it's also a bit farther from what the C was doing, which may then make it harder to cross-reference against the C code during later liftings. That said, I don't know if it makes cross-referencing the original C that much harder. The part that's more confusing is that we're inverting the control flow (e.g. if x { goto loop; } becomes if !x { break; }), which is really what makes the translated Rust hard to compare to the original C. There's a couple places in the snapshot tests where we could avoid this inversion, but in general relooper is going to have to do some amount of inversion around goto control flow.

Also, we're already doing this for ! operations, i.e. if !(!x) is getting turned into if x. If we don't want to automatically invert the conditions, maybe we also shouldn't be doing that? I don't think that'd actually improve things, which leads me to think that this simplification of negated conditionals is reasonable.

So I guess I'm mildly in favor of this. It produces cleaner code, and the drawback to it seems pretty minor.

@ahomescu ahomescu requested a review from randomPoison May 7, 2026 03:45
@Rua Rua force-pushed the negate-comparisons branch from 0fd42a5 to c2013aa Compare May 13, 2026 08:57
@ahomescu ahomescu merged commit 0bcaa74 into immunant:master May 22, 2026
11 checks passed
@Rua Rua deleted the negate-comparisons branch May 23, 2026 17:44
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.

3 participants