Skip to content

Expression enum and Root variant - #9247

Open
mhk197 wants to merge 4 commits into
developfrom
mk/root-expression
Open

Expression enum and Root variant#9247
mhk197 wants to merge 4 commits into
developfrom
mk/root-expression

Conversation

@mhk197

@mhk197 mhk197 commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

Makes Root an Expression variant instead of a ScalarFnVTable implementation.

This is groundwork for lambdas and variables — those are also nodes whose dtype comes from the
scope rather than from their children, and they need the same treatment.

Root is not a scalar function. Its dtype comes from the scope rather than from its children (it has no children) and it is not executable.

This PR makes Expression an enum instead of a struct. Root becomes a variant, and upcoming Lambda and Variable become variants as well.

pub enum Expression {
    Scalar { scalar_fn: ScalarFnRef, children: Arc<Vec<Expression>> },
    Root,
}

This is a small breaking change.

@mhk197 mhk197 changed the title root as expression variant Expression as enum instead of struct Aug 6, 2026
@mhk197 mhk197 changed the title Expression as enum instead of struct Expression enum and Root variant` Aug 6, 2026
@mhk197 mhk197 changed the title Expression enum and Root variant` Expression enum and Root variant Aug 6, 2026
@mhk197 mhk197 linked an issue Aug 6, 2026 that may be closed by this pull request
@mhk197
mhk197 force-pushed the mk/root-expression branch from 8dc4b89 to 996a4e3 Compare August 6, 2026 19:23
@mhk197
mhk197 marked this pull request as ready for review August 6, 2026 19:54
@mhk197 mhk197 closed this Aug 7, 2026
@mhk197 mhk197 reopened this Aug 7, 2026
mhk197 added 4 commits August 6, 2026 20:06
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
Signed-off-by: Matt Katz <mhkatz97@gmail.com>
@mhk197
mhk197 force-pushed the mk/root-expression branch from f17b031 to 8bd1b1f Compare August 7, 2026 03:06
@mhk197 mhk197 added changelog/chore A trivial change changelog/break A breaking API change and removed changelog/chore A trivial change labels Aug 7, 2026
@codspeed-hq

codspeed-hq Bot commented Aug 7, 2026

Copy link
Copy Markdown

Merging this PR will degrade performance by 10.68%

⚠️ Different runtime environments detected

Some benchmarks with significant performance changes were compared across different runtime environments,
which may affect the accuracy of the results.

Open the report in CodSpeed to investigate

❌ 1 regressed benchmark
✅ 1928 untouched benchmarks
⏩ 51 skipped benchmarks1

Warning

Please fix the performance issues or acknowledge them on CodSpeed.

Performance Changes

Mode Benchmark BASE HEAD Efficiency
Simulation compress_fsst[(10000, 64, 8)] 9.6 ms 10.7 ms -10.68%

Tip

Investigate this regression by commenting @codspeedbot fix this regression on this PR, or directly use the CodSpeed MCP with your agent.


Comparing mk/root-expression (8bd1b1f) with develop (4e3be5b)

Open in CodSpeed

Footnotes

  1. 51 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

changelog/break A breaking API change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Epic: Support Higher Order Functions

1 participant