Skip to content

Support functions with multiple outputs (EliminateMultipleOutputs pass)#18

Draft
keyboardDrummer-bot wants to merge 4 commits intomainfrom
extract-eliminate-multiple-outputs
Draft

Support functions with multiple outputs (EliminateMultipleOutputs pass)#18
keyboardDrummer-bot wants to merge 4 commits intomainfrom
extract-eliminate-multiple-outputs

Conversation

@keyboardDrummer-bot
Copy link
Copy Markdown
Collaborator

@keyboardDrummer-bot keyboardDrummer-bot commented Apr 21, 2026

Extracted from #16 as requested by @keyboardDrummer.

Summary

Adds support for functions with multiple output parameters by introducing the EliminateMultipleOutputs pass.

Changes

  1. EliminateMultipleOutputs pass (Strata/Languages/Laurel/EliminateMultipleOutputs.lean): Transforms functional procedures (.isFunctional = true) with multiple outputs into procedures that return a single synthesized result datatype. Call sites are rewritten to destructure the result using generated accessors. For a function foo(x: int) returns (a: int, b: int), the pass:

    • Generates a foo$result datatype with fields out0, out1
    • Rewrites foo to return foo$result
    • Rewrites Assign call sites to use a temp variable + destructor calls
    • Rewrites LocalVariable call sites (single-target) to extract the first output
  2. Pipeline integration: The pass is called in runLaurelPasses after constrained type elimination, followed by a re-resolution step.

  3. Test: Added T22_MultipleOutputs.lean with a test for a function with multiple outputs, using a single-target assignment as the LHS.

Note: The LocalVariable AST node is unchanged — it retains its original (name : Identifier) (type : AstNode HighType) (initializer : ...) form. Multi-output functions are handled entirely through the EliminateMultipleOutputs pass without modifying the AST.

- Change LocalVariable from (name, type, init) to (List Parameter, init)
  to support multi-output destructuring
- Add EliminateMultipleOutputs pass that transforms functional procedures
  with multiple outputs into procedures returning a synthesized result
  datatype, rewriting call sites to destructure via generated accessors
- The pass checks .isFunctional to determine which procedures to handle
- Add pass call in runLaurelPasses after constrained type elimination
- Add test for functions with multiple outputs
- Update all pattern matches across the codebase (~18 files)
@keyboardDrummer
Copy link
Copy Markdown
Owner

@keyboardDrummer-bot please remove from this PR the change LocalVariable AST change: Changed StmtExpr.LocalVariable from (name : Identifier) (type : AstNode HighType) (initializer : ...) to (parameters : List Parameter) (initializer : ...). This enables multi-output destructuring where a single LocalVariable node can declare multiple variables initialized from a multi-output call.

We can have functions with multiple outputs without supporting locals with multiple targets. Just use a destructive assignment in the test as a LHS for the function with multiple outputs

…er) form

Reverts the LocalVariable AST change from (parameters : List Parameter)
back to (name : Identifier) (type : AstNode HighType) (initializer : ...).

The EliminateMultipleOutputs pass now works with the original LocalVariable
form. It rewrites both multi-target Assign and single-target LocalVariable/
Assign call sites to use the synthesized result datatype destructors.

The test uses a destructuring assignment (single-target assign) as the LHS
for the multi-output function call.
@keyboardDrummer-bot keyboardDrummer-bot force-pushed the extract-eliminate-multiple-outputs branch from 106588f to d8fa4d4 Compare April 21, 2026 14:18
@github-actions github-actions Bot removed the Python label Apr 21, 2026
@keyboardDrummer-bot
Copy link
Copy Markdown
Collaborator Author

Done. I've reverted the LocalVariable AST change — it's back to the original (name : Identifier) (type : AstNode HighType) (initializer : ...) form.

Changes made:

  • Reverted all 14 files that had pattern match updates for the List Parameter form back to their main branch versions
  • Updated EliminateMultipleOutputs.lean to work with the original LocalVariable form:
    • Rewrites multi-target Assign call sites (created by internal passes) to use temp variable + destructors
    • Also rewrites single-target LocalVariable and Assign call sites that call a multi-output function, extracting the first output via the destructor
    • Removed the LocalVariable multi-target rewriting case (no longer possible with the original AST)
  • Updated the test to use a single-target assignment (a := twoOutputs(5)) as the LHS for the multi-output function call

All tests pass (the only failure is the pre-existing StrataTest.DDM.Integration.Java.TestGen due to a missing jar file).


procedure testMultiOut() {
var a: int;
a := twoOutputs(5);
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keyboardDrummer-bot
Why is this not throwing an error? twoOutputs MUST be assigned using two LHS targets. If the number of LHS and RHS elements does not match, this should emit an error.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed. The EliminateMultipleOutputs pass now validates that the number of LHS targets matches the number of function outputs before transforming. When there's a mismatch (like assigning a 2-output function to 1 target), it emits an error and skips the transformation entirely.

The test now expects:

  a := twoOutputs(5)
//     ^^^^^^^^^^^^^ error: call to 'twoOutputs' has 1 assignment target(s), but the function returns 2 output(s)

All existing tests pass (only pre-existing TestGen jar failure).

The EliminateMultipleOutputs pass now validates that the number of
assignment targets matches the number of outputs for multi-output
function calls. When there is a mismatch (e.g., assigning a 2-output
function to a single variable), the pass emits a diagnostic error and
skips the transformation entirely.

Changes:
- Added validation phase that scans all call sites before transforming
- eliminateMultipleOutputs now returns Program × List DiagnosticModel
- Pipeline collects and reports the new diagnostics
- Updated test to expect the mismatch error
function twoOutputs(x: int)
returns (a: int, b: int);

procedure testMultiOut() {
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@keyboardDrummer-bot Can you also add a test, which will need to be in another program, where there is no error?

@github-actions github-actions Bot added the Git conflicts PR has merge conflicts with the base branch label Apr 22, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Git conflicts PR has merge conflicts with the base branch Laurel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants