Conversation
|
Is this AI generated? This looks AI generated: if that is the case, you should indicate it clearly. |
Yes, I've updated the description. |
|
I'm sorry but I don't see myself reviewing an AI generated PR with a diff of 6k lines and barely any documentation: I have no idea how your code works. I'm not opposed to AI assisted code and I welcome improvements to the toolchain, but in the current state I can not reasonably review this. Could you give me detailed explanations about how the PR works, the choices you've made, etc.? |
Yes, I figured its too big to accept in one PR, especially for first time contributor, but it also address the existing limitation holistically (if not we should construct a test case where it doesn't work). One way to see the logic of this change is to critique the plan from first commit. Another is the following detailed explanation (also AI generated but with my edits). For code review I'd recommend to review the implementation files first, skip generated backend files, that large generated diff is mostly from the generated Lean import opening LoopExit. Also important note: I didn't test this on anything that requires Windows. What this PR changesThe PR adds support for nested loop exits that leave the current loop:
Previously the pre-pass rejected or rewrote many of these cases. This PR stops treating them as unsupported and instead represents them explicitly in the symbolic and pure ASTs. Core ideaA normal loop is still translated using: ControlFlow.cont state
ControlFlow.done resultBut if a loop body can exit beyond the current loop, the done payload needs to say which kind of exit happened. The PR introduces: LoopExit normalBreak propagatedBreak propagatedContinue propagatedReturn Meaning:
Panic/failure is not encoded in LoopExit; it remains the existing Result.fail path. Example shape: loop (...) : Result (LoopExit normalBreakTy propagatedBreakTy Unit Unit) The caller then matches on the LoopExit after the loop and either continues normal execution or propagates the exit outward. Depth handlingLLBC break/continue depths are relative to the loop where they occur. Inside the current loop:
The decrement is important: once an exit leaves the current loop, one loop boundary has been consumed. Main implementation pathRecommended review order:
Why introduce AeneasSumLoopExit has one field for propagated breaks and one field for propagated continues. If multiple break depths reach the same loop, we need to distinguish them inside that one field. For example, a loop may receive both:
Those are both propagated breaks, but with different remaining depths. The PR encodes that payload as a binary sum: AeneasSum leftPayload rightPayload This avoids generating a custom ADT per loop shape. Lean/Coq/F* use an Aeneas-defined sum to avoid naming collisions; HOL4 uses its native sum with INL/INR. Why not keep using only ControlFlowControlFlow only distinguishes continue vs done. Once done may mean “normal break”, “break outer”, “continue outer”, or “return”, we need another discriminator. Encoding all of that into tuples would be fragile and proof-hostile; LoopExit makes the generated shape explicit. Current limitationsThis PR does not try to solve every possible loop shape. In particular, fixed-point loops with propagated exits but no normal break are still rejected. Most while loops have a normal break path through the false condition, so this still covers the intended nested-control cases. Example: an infinite loop where every exit is a return or an outer control-flow jump, and there is no path that simply breaks the current loop. pub fn no_normal_break(flag: bool) -> u32 {
loop {
if flag {
return 1;
} else {
return 2;
}
}
}HOL4 extraction names are wired, but the new HOL4 nested-loop fixture is not regenerated in this PR because the current test setup does not regenerate HOL4 outputs for these cases. Test coverageThe tests cover the implementation at several levels:
|
Summary
returninside nested loops and labelled outerbreak/continue.lowering, and backend extraction.
Testing
recursive loop extraction, multi-depth breaks, outer continues, and backend-specific output
shapes.
context collection, and pure/backend extraction naming.
fixtures to pin emitted syntax.
Development