Skip to content

Plan checked ASTs without wrapper conversion - #924

Merged
snazy merged 2 commits into
projectnessie:mainfrom
snazy:checked-ast-no-wrapper-conv
Jul 20, 2026
Merged

Plan checked ASTs without wrapper conversion#924
snazy merged 2 commits into
projectnessie:mainfrom
snazy:checked-ast-no-wrapper-conv

Conversation

@snazy

@snazy snazy commented Jul 20, 2026

Copy link
Copy Markdown
Member

Pass checked expression metadata directly into the interpreter planner during program creation.

This avoids constructing a temporary CheckedExpr on the hot compile-to-program path while preserving the existing checked-expression API.

Pass checked expression metadata directly into the interpreter planner during program creation.

This avoids constructing a temporary CheckedExpr on the hot compile-to-program path while preserving the existing checked-expression API.
@snazy
snazy requested a review from XN137 July 20, 2026 10:08
XN137
XN137 previously approved these changes Jul 20, 2026
Interpretable newInterpretable(CheckedExpr checked, InterpretableDecorator... decorators);

/**
* NewInterpretable creates an Interpretable from a checked expression and its metadata without

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: newInterpretable (but maybe javadocs shouldnt repeat the method name at all)

return p.plan(checked.getExpr());
}

/** NewIntepretable implements the Interpreter interface method. */

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: typo Intepret and lowercase n
(looks like the Intepretable typo exists below as well)

// When the AST has been checked it contains metadata that can be used to speed up program
// execution.
CheckedExpr checked = astToCheckedExpr(ast);
p.interpretable = p.interpreter.newInterpretable(checked, decs);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

is there still a valid use case for the old method? should we remove or deprecate it?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The existing CheckedExpr overload appears useful, it has direct callers, and is the abstract compatibility point for external Interpreter implementations (if those exist 🤷). The new overload is deliberately a default method.

@snazy
snazy merged commit fd7203a into projectnessie:main Jul 20, 2026
3 checks passed
@snazy
snazy deleted the checked-ast-no-wrapper-conv branch July 20, 2026 16:17
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.

2 participants