BE-615: HashQL: Introduce the concept of synthetic closures in the MIR#8894
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
PR SummaryMedium Risk Overview Administrative reduction gains Intrinsics move from Supporting changes: Note: Some postgres Reviewed by Cursor Bugbot for commit c836d70. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## bm/be-607-hashql-remove-smallvec-from-the-standardlibrary-for-module #8894 +/- ##
=========================================================================================================
+ Coverage 24.83% 40.65% +15.81%
=========================================================================================================
Files 663 791 +128
Lines 50118 70641 +20523
Branches 3379 3992 +613
=========================================================================================================
+ Hits 12448 28719 +16271
- Misses 37513 41590 +4077
- Partials 157 332 +175
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
d08cc92 to
5014ae0
Compare
ac8ed9e to
4aacfd8
Compare
5014ae0 to
8c7419a
Compare
4aacfd8 to
f2b45cb
Compare
| // In this case it may either be a forwarding closure **or** a synthetic closure, this | ||
| // depends on the final statement, in either case the last statement must be an assignment. | ||
| let StatementKind::Assign(Assign { lhs, ref rhs }) = final_stmt.kind else { | ||
| return None; | ||
| }; | ||
|
|
||
| // The kind depends on the final statement. Both cases are beta-reduction | ||
| // (substituting arguments into a function body at a call site): | ||
| // | ||
| // Apply: the body forwards to another call. The wrapper (lambda x. f(x)) is an | ||
| // eta-expansion of the inner function; inlining at the call site is beta-reduction | ||
| // that exposes the inner call directly. | ||
| // | ||
| // Binary, Unary, and other primitive operations: the body computes a single result | ||
| // from its arguments. Inlining substitutes the arguments and exposes the primitive | ||
| // operation. A subsequent constant folding pass may then perform delta-reduction | ||
| // (evaluating the primitive when operands are known). | ||
| let kind = match rhs { | ||
| RValue::Apply(_) => Self::ForwardingClosure, | ||
| RValue::Aggregate(_) | ||
| | RValue::Binary(_) | ||
| | RValue::Unary(_) | ||
| | RValue::Input(_) | ||
| | RValue::Load(_) => Self::TrivialClosure, | ||
| }; |
| if bb.terminator.kind | ||
| == TerminatorKind::Return(Return { | ||
| value: Operand::Place(lhs), | ||
| }) | ||
| { |
| // `property<T>(entity: Entity<T>, path: JsonPath) -> ?` | ||
| // TODO(BE-62): return `Option<?>` once pattern matching allows for option destructuring, to | ||
| // allow for proper comparison | ||
| let decl = decl!(context; | ||
| <T>(entity: context.ty.apply([(entity_ty.arguments[0].id, T)], entity_ty.id), | ||
| path: json_path_ty.id | ||
| ) -> option(&context.ty, context.ty.unknown()) | ||
| ) -> context.ty.unknown() | ||
| ); |
f2b45cb to
489dd1a
Compare
8c7419a to
4abefab
Compare
4abefab to
9088654
Compare
489dd1a to
5628792
Compare
| impl DefId { | ||
| /// Built-in dictionary insert operation (immutable). | ||
| /// | ||
| /// This operation inserts a key-value pair into a dictionary, | ||
| /// returning a new dictionary with the added pair. The original | ||
| /// dictionary remains unchanged. | ||
| pub const DICT_INSERT: Self = Self::new(0xFFFF_FE00); | ||
| /// Built-in dictionary insert operation (mutable). | ||
| /// | ||
| /// This operation inserts a key-value pair into a dictionary in-place, | ||
| /// modifying the original dictionary. Used for efficient dictionary | ||
| /// construction and updates. | ||
| pub const DICT_INSERT_MUT: Self = Self::new(0xFFFF_FE01); | ||
| /// Built-in dictionary remove operation (immutable). | ||
| /// | ||
| /// This operation removes a key-value pair from a dictionary, | ||
| /// returning a new dictionary without the specified key. The | ||
| /// original dictionary remains unchanged. | ||
| pub const DICT_REMOVE: Self = Self::new(0xFFFF_FE02); | ||
| /// Built-in dictionary remove operation (mutable). | ||
| /// | ||
| /// This operation removes a key-value pair from a dictionary in-place, | ||
| /// modifying the original dictionary and returning the removed value | ||
| /// if the key existed. | ||
| pub const DICT_REMOVE_MUT: Self = Self::new(0xFFFF_FE03); | ||
| /// Built-in list pop operation (immutable). | ||
| /// | ||
| /// This operation removes the last element from a list, returning | ||
| /// both the element and a new list without the element. The original | ||
| /// list remains unchanged. | ||
| pub const LIST_POP: Self = Self::new(0xFFFF_FE04); | ||
| /// Built-in list pop operation (mutable). | ||
| /// | ||
| /// This operation removes the last element from a list in-place, | ||
| /// returning the removed element while modifying the original list. | ||
| pub const LIST_POP_MUT: Self = Self::new(0xFFFF_FE05); | ||
| /// Built-in list push operation (immutable). | ||
| /// | ||
| /// This operation appends an element to a list, returning a new list | ||
| /// without modifying the original. Used for functional-style list | ||
| /// manipulation where immutability is preferred. | ||
| pub const LIST_PUSH: Self = Self::new(0xFFFF_FE06); | ||
| /// Built-in list push operation (mutable). | ||
| /// | ||
| /// This operation appends an element to a list in-place, modifying | ||
| /// the original list. Used for imperative-style list manipulation | ||
| /// where performance is critical. | ||
| pub const LIST_PUSH_MUT: Self = Self::new(0xFFFF_FE07); | ||
| pub const PLACEHOLDER: Self = Self::MAX; | ||
| } |
| let inline = match body.source { | ||
| Source::Ctor(_) => InlineDirective::Always, | ||
| Source::Ctor(_) | Source::Synthetic(_) => InlineDirective::Always, | ||
| Source::Closure(_, _) | Source::Thunk(_, _) => InlineDirective::Heuristic, | ||
| Source::Intrinsic(_) | Source::GraphReadFilter(_) => InlineDirective::Never, | ||
| }; |
| pub(in crate::module::std_lib) mod bits; | ||
| pub(in crate::module::std_lib) mod bool; | ||
| pub(in crate::module::std_lib) mod cmp; | ||
| pub(in crate::module::std_lib) mod json; | ||
| pub mod json; | ||
| pub(in crate::module::std_lib) mod math; | ||
| pub mod option; | ||
| pub(in crate::module::std_lib) mod result; |
| // `property<T>(entity: Entity<T>, path: JsonPath) -> ?` | ||
| // TODO(BE-62): return `Option<?>` once pattern matching allows for option destructuring, to | ||
| // allow for proper comparison | ||
| let decl = decl!(context; | ||
| <T>(entity: context.ty.apply([(entity_ty.arguments[0].id, T)], entity_ty.id), | ||
| path: json_path_ty.id | ||
| ) -> option(&context.ty, context.ty.unknown()) | ||
| ) -> context.ty.unknown() | ||
| ); |
| // In this case it may either be a forwarding closure **or** a synthetic closure, this | ||
| // depends on the final statement, in either case the last statement must be an assignment. |
| (@source intrinsic) => { | ||
| $crate::body::Source::Intrinsic($crate::def::DefId::PLACEHOLDER) | ||
| $crate::body::Source::Intrinsic($crate::intrinsic::Intrinsic { id: $crate::intrinsic::IntrinsicId::EntityPropertyAccess, optimize: true }) | ||
| }; |
| // In this case it may either be a forwarding closure **or** a synthetic closure, this | ||
| // depends on the final statement, in either case the last statement must be an assignment. | ||
| let StatementKind::Assign(Assign { lhs, ref rhs }) = final_stmt.kind else { | ||
| return None; | ||
| }; |
| Source::Intrinsic(Intrinsic { id, .. }) => { | ||
| write!(self.line_buffer, "{{intrinsic#{id}}}") | ||
| } |
| impl DefId { | ||
| /// Built-in dictionary insert operation (immutable). | ||
| /// | ||
| /// This operation inserts a key-value pair into a dictionary, | ||
| /// returning a new dictionary with the added pair. The original | ||
| /// dictionary remains unchanged. | ||
| pub const DICT_INSERT: Self = Self::new(0xFFFF_FE00); | ||
| /// Built-in dictionary insert operation (mutable). | ||
| /// | ||
| /// This operation inserts a key-value pair into a dictionary in-place, | ||
| /// modifying the original dictionary. Used for efficient dictionary | ||
| /// construction and updates. | ||
| pub const DICT_INSERT_MUT: Self = Self::new(0xFFFF_FE01); | ||
| /// Built-in dictionary remove operation (immutable). | ||
| /// | ||
| /// This operation removes a key-value pair from a dictionary, | ||
| /// returning a new dictionary without the specified key. The | ||
| /// original dictionary remains unchanged. | ||
| pub const DICT_REMOVE: Self = Self::new(0xFFFF_FE02); | ||
| /// Built-in dictionary remove operation (mutable). | ||
| /// | ||
| /// This operation removes a key-value pair from a dictionary in-place, | ||
| /// modifying the original dictionary and returning the removed value | ||
| /// if the key existed. | ||
| pub const DICT_REMOVE_MUT: Self = Self::new(0xFFFF_FE03); | ||
| /// Built-in list pop operation (immutable). | ||
| /// | ||
| /// This operation removes the last element from a list, returning | ||
| /// both the element and a new list without the element. The original | ||
| /// list remains unchanged. | ||
| pub const LIST_POP: Self = Self::new(0xFFFF_FE04); | ||
| /// Built-in list pop operation (mutable). | ||
| /// | ||
| /// This operation removes the last element from a list in-place, | ||
| /// returning the removed element while modifying the original list. | ||
| pub const LIST_POP_MUT: Self = Self::new(0xFFFF_FE05); | ||
| /// Built-in list push operation (immutable). | ||
| /// | ||
| /// This operation appends an element to a list, returning a new list | ||
| /// without modifying the original. Used for functional-style list | ||
| /// manipulation where immutability is preferred. | ||
| pub const LIST_PUSH: Self = Self::new(0xFFFF_FE06); | ||
| /// Built-in list push operation (mutable). | ||
| /// | ||
| /// This operation appends an element to a list in-place, modifying | ||
| /// the original list. Used for imperative-style list manipulation | ||
| /// where performance is critical. | ||
| pub const LIST_PUSH_MUT: Self = Self::new(0xFFFF_FE07); | ||
| pub const PLACEHOLDER: Self = Self::MAX; | ||
| } |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 3c04155. Configure here.
| FROM "entity_editions" AS "ee" | ||
| WHERE "ee"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id") AS "entity_editions_0_0_1" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::boolean), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" | ||
| >>>>>>> d821432937 (feat: add to different passes) |
There was a problem hiding this comment.
Unresolved merge conflict markers
High Severity
Two postgres compiletest expected-output files still contain unresolved merge conflict markers (<<<<<<< HEAD, =======, >>>>>>> …). Snapshot or stdout comparison tests will fail until one side of each conflict is chosen and the markers are removed.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 3c04155. Configure here.
|
|
||
| SELECT ("continuation_0_0"."row")."block" AS "continuation_0_0_block", ("continuation_0_0"."row")."locals" AS "continuation_0_0_locals", ("continuation_0_0"."row")."values" AS "continuation_0_0_values", ("continuation_1_0"."row")."block" AS "continuation_1_0_block", ("continuation_1_0"."row")."locals" AS "continuation_1_0_locals", ("continuation_1_0"."row")."values" AS "continuation_1_0_values" | ||
| FROM "entity_temporal_metadata" AS "entity_temporal_metadata_0_0_0" | ||
| <<<<<<< HEAD |
|
|
||
| SELECT ("continuation_1_0"."row")."block" AS "continuation_1_0_block", ("continuation_1_0"."row")."locals" AS "continuation_1_0_locals", ("continuation_1_0"."row")."values" AS "continuation_1_0_values" | ||
| FROM "entity_temporal_metadata" AS "entity_temporal_metadata_0_0_0" | ||
| <<<<<<< HEAD |
| <<<<<<< HEAD | ||
| INNER JOIN "entity_editions" AS "entity_editions_0_0_1" | ||
| ON "entity_editions_0_0_1"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::bool), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_0_0" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((to_jsonb("entity_temporal_metadata_0_0_0"."entity_uuid") = to_jsonb(($3::jsonb)))::bool), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" | ||
| ======= | ||
| CROSS JOIN LATERAL (SELECT "ee"."entity_edition_id" AS "entity_edition_id", "ee"."properties" AS "properties", "ee"."archived" AS "archived", "ee"."confidence" AS "confidence", "ee"."provenance" AS "provenance", "ee"."property_metadata" AS "property_metadata" | ||
| FROM "entity_editions" AS "ee" | ||
| WHERE "ee"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id") AS "entity_editions_0_0_1" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::boolean), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_0_0" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((to_jsonb("entity_temporal_metadata_0_0_0"."entity_uuid") = to_jsonb(($3::jsonb)))::boolean), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" | ||
| >>>>>>> d821432937 (feat: add to different passes) |
| <<<<<<< HEAD | ||
| INNER JOIN "entity_editions" AS "entity_editions_0_0_1" | ||
| ON "entity_editions_0_0_1"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::bool), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" | ||
| ======= | ||
| CROSS JOIN LATERAL (SELECT "ee"."entity_edition_id" AS "entity_edition_id", "ee"."properties" AS "properties", "ee"."archived" AS "archived", "ee"."confidence" AS "confidence", "ee"."provenance" AS "provenance", "ee"."property_metadata" AS "property_metadata" | ||
| FROM "entity_editions" AS "ee" | ||
| WHERE "ee"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id") AS "entity_editions_0_0_1" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::boolean), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" | ||
| >>>>>>> d821432937 (feat: add to different passes) |
| impl DefId { | ||
| /// Built-in dictionary insert operation (immutable). | ||
| /// | ||
| /// This operation inserts a key-value pair into a dictionary, | ||
| /// returning a new dictionary with the added pair. The original | ||
| /// dictionary remains unchanged. | ||
| pub const DICT_INSERT: Self = Self::new(0xFFFF_FE00); | ||
| /// Built-in dictionary insert operation (mutable). | ||
| /// | ||
| /// This operation inserts a key-value pair into a dictionary in-place, | ||
| /// modifying the original dictionary. Used for efficient dictionary | ||
| /// construction and updates. | ||
| pub const DICT_INSERT_MUT: Self = Self::new(0xFFFF_FE01); | ||
| /// Built-in dictionary remove operation (immutable). | ||
| /// | ||
| /// This operation removes a key-value pair from a dictionary, | ||
| /// returning a new dictionary without the specified key. The | ||
| /// original dictionary remains unchanged. | ||
| pub const DICT_REMOVE: Self = Self::new(0xFFFF_FE02); | ||
| /// Built-in dictionary remove operation (mutable). | ||
| /// | ||
| /// This operation removes a key-value pair from a dictionary in-place, | ||
| /// modifying the original dictionary and returning the removed value | ||
| /// if the key existed. | ||
| pub const DICT_REMOVE_MUT: Self = Self::new(0xFFFF_FE03); | ||
| /// Built-in list pop operation (immutable). | ||
| /// | ||
| /// This operation removes the last element from a list, returning | ||
| /// both the element and a new list without the element. The original | ||
| /// list remains unchanged. | ||
| pub const LIST_POP: Self = Self::new(0xFFFF_FE04); | ||
| /// Built-in list pop operation (mutable). | ||
| /// | ||
| /// This operation removes the last element from a list in-place, | ||
| /// returning the removed element while modifying the original list. | ||
| pub const LIST_POP_MUT: Self = Self::new(0xFFFF_FE05); | ||
| /// Built-in list push operation (immutable). | ||
| /// | ||
| /// This operation appends an element to a list, returning a new list | ||
| /// without modifying the original. Used for functional-style list | ||
| /// manipulation where immutability is preferred. | ||
| pub const LIST_PUSH: Self = Self::new(0xFFFF_FE06); | ||
| /// Built-in list push operation (mutable). | ||
| /// | ||
| /// This operation appends an element to a list in-place, modifying | ||
| /// the original list. Used for imperative-style list manipulation | ||
| /// where performance is critical. | ||
| pub const LIST_PUSH_MUT: Self = Self::new(0xFFFF_FE07); | ||
| pub const PLACEHOLDER: Self = Self::MAX; | ||
| } |
| <<<<<<< HEAD | ||
| INNER JOIN "entity_editions" AS "entity_editions_0_0_1" | ||
| ON "entity_editions_0_0_1"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::bool), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_0_0" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((to_jsonb("entity_temporal_metadata_0_0_0"."entity_uuid") = to_jsonb(($3::jsonb)))::bool), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" |
| <<<<<<< HEAD | ||
| INNER JOIN "entity_editions" AS "entity_editions_0_0_1" | ||
| ON "entity_editions_0_0_1"."entity_edition_id" = "entity_temporal_metadata_0_0_0"."entity_edition_id" | ||
| CROSS JOIN LATERAL (SELECT (ROW(COALESCE(((NOT("entity_editions_0_0_1"."archived"))::bool), FALSE), NULL, NULL, NULL)::continuation) AS "row") AS "continuation_1_0" | ||
| ======= |
| // In this case it may either be a forwarding closure **or** a synthetic closure, this | ||
| // depends on the final statement, in either case the last statement must be an assignment. | ||
| let StatementKind::Assign(Assign { lhs, ref rhs }) = final_stmt.kind else { |



🌟 What is the purpose of this PR?
Introduces
Source::Syntheticfor compiler-generated wrapper bodies and extends administrative reduction to handle single-operation function bodies (TrivialClosure).This is groundwork for making intrinsics usable in value position (e.g. passing
+as an argument to a higher-order function). TheSyntheticsource variant will identify wrapper bodies that bridge non-first-class operations into callable closures, andTrivialClosureensures they are always inlined away.🔍 What does this change?
Source::Synthetic(Symbol)to the MIRSourceenum for compiler-synthesized operation wrappersReductionKind::TrivialClosureto administrative reduction, recognizing single-basic-block bodies where a trivial prelude leads to a single non-call operation (Binary,Unary,Aggregate,Input,Load) whose result is returnedSyntheticbodies getInlineDirective::Always(same asCtor)Sourcematches updated: execution pipeline, statement placement, eval context, pretty printer[synthetic sym::path]source to thebody!test macroThe
TrivialClosureclassification is intentionally broader than just synthetic bodies. Any function matching the shape is reducible, which unlocks optimization cascades: single-operation closures get inlined, exposing constant-foldable expressions, enabling branch elimination downstream.Pre-Merge Checklist 🚀
🚢 Has this modified a publishable library?
This PR:
📜 Does this require a change to the docs?
The changes in this PR:
🕸️ Does this require a change to the Turbo Graph?
The changes in this PR:
🐾 Next steps
qualified_pathin reification to generate synthetic wrapper bodies for intrinsics in value position🛡 What tests cover this?
classify_trivial_closure_binary,classify_trivial_closure_unary,classify_non_reducible_non_trivial_preludeinline_trivial_closure_binary,inline_trivial_closure_transitivenested-branch-eliminationnow demonstrates the full optimization cascade)❓ How to test this?
cargo test --package hashql-mir --lib -- administrative_reduction::tests📹 Demo
The
nested-branch-eliminationcompiletest shows the cascade effect: closures containing single comparisons (> 100,< 0) are now inlined by AR, exposing constant-foldable comparisons (50 > 100,50 < 0), which enables the entire branch structure to collapse toreturn "in range".