-
-
Notifications
You must be signed in to change notification settings - Fork 158
perf(hir): keep the array type through methods chained on new Array<T>(n) #9036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| Kept the inferred array type through builtin methods chained on `new Array<T>(n)` — `new Array<number>(n).fill(0)` now infers `number[]` instead of `Any`, so index stores on such arrays take the inline guarded lane (34.6 → 3.4 ns per store). | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1236,6 +1236,25 @@ pub(crate) fn infer_call_return_type(callee: &ast::Expr, ctx: &LoweringContext) | |
| return Type::Any; | ||
| } | ||
| let obj_ty = infer_type_from_expr(&member.obj, ctx); | ||
| // `new Array<T>(n)` types as `Generic { base: "Array", [T] }` | ||
| // (the explicit-type-args early return above) — the same array | ||
| // `T[]` denotes, and downstream consumers already treat the | ||
| // two as one (`typed_parse.rs`, `stmt_loops.rs`, | ||
| // `element_shape_loop.rs`). This table must too: without the | ||
| // rebind, `new Array<number>(n).fill(0)` inferred `Any`, the | ||
| // binding lost its array type, and every later `a[i] = v` on | ||
| // it took the generic feedback store instead of the inline | ||
| // guarded lane — 34.6 vs 3.7 ns per store, for the array's | ||
| // whole life. | ||
| let obj_ty = match obj_ty { | ||
| Type::Generic { | ||
| ref base, | ||
| ref type_args, | ||
| } if base == "Array" && type_args.len() == 1 => { | ||
| Type::Array(Box::new(type_args[0].clone())) | ||
| } | ||
|
Comment on lines
+1253
to
+1255
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift 🔎 Supported by static analysis🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- applicable repository guidance ---'
find /tmp/coderabbit-repo-knowledge/perryts-perry-d4a878bc -maxdepth 2 -type f -name '*.md' -print
printf '%s\n' '--- lower_types.rs target ---'
sed -n '1225,1270p' crates/perry-hir/src/lower_types.rs
sed -n '1325,1380p' crates/perry-hir/src/lower_types.rs
printf '%s\n' '--- relevant definitions and tests ---'
rg -n -C 3 'Type::Array|flatMap|map|new Array|Generic' crates/perry-hir/src/lower_types.rs crates -g '*.rs' -g '*.ts' -g '*.tsx' | head -240
printf '%s\n' '--- scoped guidance and learnings ---'
for f in /tmp/coderabbit-repo-knowledge/perryts-perry-d4a878bc/*/*.md; do
case "$f" in
*/learnings/*|*/architecture/*) ;;
*) printf '\n### %s\n' "$f"; cat "$f" ;;
esac
doneRepository: PerryTS/perry Length of output: 29915 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- complete member-call inference path ---'
sed -n '1110,1395p' crates/perry-hir/src/lower_types.rs
printf '%s\n' '--- downstream use of inferred call types ---'
rg -n -C 4 'infer_type_from_expr\(|method_return_may_depend_on_receiver_type|return_type|Type::Array' crates/perry-hir/src crates/perry-codegen/src -g '*.rs' | head -260
printf '%s\n' '--- HIR-specific learning ---'
cat /tmp/coderabbit-repo-knowledge/perryts-perry-d4a878bc/learnings/crates-perry-hir-src.mdRepository: PerryTS/perry Length of output: 35458 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- exact callers of infer_call_return_type ---'
rg -n -C 8 'infer_call_return_type' crates/perry-hir/src crates/perry-codegen/src -g '*.rs'
printf '%s\n' '--- array method lowering and callback handling ---'
rg -n -C 8 'ArrayLikeMethod|ArrayMap|ArrayFlatMap|flat_map|method_name.*map|method.*flatMap' crates/perry-hir/src -g '*.rs' | head -260Repository: PerryTS/perry Length of output: 33672 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- construction of specialized array HIR ---'
rg -n -C 12 'ArrayMap|ArrayFlatMap|ArrayLikeMethod' crates/perry-hir/src/lower crates/perry-hir/src/lower_expr.rs crates/perry-hir/src -g '*.rs' | head -320
printf '%s\n' '--- call lowering entry points ---'
rg -n -C 10 'Call\(call\)|Member\(member\)|method_name|callee' crates/perry-hir/src/lower -g '*.rs' | head -300Repository: PerryTS/perry Length of output: 50369 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- array-fold dispatch ---'
sed -n '1,125p' crates/perry-hir/src/lower/array_fold.rs
printf '%s\n' '--- expression call lowering dispatch ---'
rg -n -C 12 'array_fold|try_array|lower_array|property.*map|ArrayLikeMethod' crates/perry-hir/src/lower/expr_call.rs crates/perry-hir/src/lower -g '*.rs' | head -220Repository: PerryTS/perry Length of output: 27490 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- consumers that assign inferred types to lowered calls ---'
rg -n -C 10 'infer_type_from_expr' crates/perry-hir/src/lower -g '*.rs' | head -320
printf '%s\n' '--- array-only method result lowering ---'
sed -n '331,390p' crates/perry-hir/src/lower/expr_call/array_only_methods.rs
sed -n '560,720p' crates/perry-hir/src/lower/expr_call/array_only_methods.rsRepository: PerryTS/perry Length of output: 32520 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- declaration type inference ---'
rg -n -C 12 'fn infer_decl_type|infer_type_from_expr|initializer|init' crates/perry-hir/src/lower/destructuring/var_decl crates/perry-hir/src/lower/stmt.rs crates/perry-hir/src/lower/expr_call -g '*.rs' | head -360Repository: PerryTS/perry Length of output: 33565 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- locate declaration inference implementation ---'
fd -t f 'type_infer.rs|var_decl' crates/perry-hir/src
printf '%s\n' '--- statement-level declaration typing ---'
rg -n -C 14 'infer_decl_type|define_local\(|lookup_local_type|ty:.*infer' crates/perry-hir/src/lower/stmt.rs crates/perry-hir/src/destructuring -g '*.rs' | head -360Repository: PerryTS/perry Length of output: 35430 🏁 Script executed: #!/bin/bash
set -eu
cat -n crates/perry-hir/src/destructuring/var_decl/type_infer.rsRepository: PerryTS/perry Length of output: 15165 Do not preserve the receiver element type for element-changing methods
🤖 Prompt for AI Agents |
||
| other => other, | ||
| }; | ||
|
|
||
| // Phase 4.1: user class methods. When the receiver is typed | ||
| // as `Type::Named(C)` (e.g., a local declared as `p: Point` or | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,70 @@ | ||
| //! `new Array<T>(n)` types as `Generic { base: "Array", type_args: [T] }` | ||
| //! (the explicit-type-args early return in `infer_type_from_expr`), while the | ||
| //! builtin array method-return table matched only `Type::Array`. A chained | ||
| //! receiver-returning method — `new Array<number>(n).fill(0)`, the canonical | ||
| //! preallocation idiom — therefore inferred `Any`, the binding lost its array | ||
| //! type, and every later `a[i] = v` on it took the generic feedback store | ||
| //! instead of the inline guarded lane: 34.6 vs 3.7 ns per store, for the | ||
| //! array's whole life. The rebind in `lower_types.rs` normalizes the Generic | ||
| //! spelling to `Type::Array` before the method tables; these tests pin it. | ||
|
|
||
| use perry_diagnostics::SourceCache; | ||
| use perry_hir::types::Type; | ||
| use perry_hir::{lower_module, Module, Stmt}; | ||
| use perry_parser::parse_typescript_with_cache; | ||
|
|
||
| fn lower_src(src: &str) -> Module { | ||
| let src = src.to_string(); | ||
| std::thread::Builder::new() | ||
| .stack_size(32 * 1024 * 1024) | ||
| .spawn(move || { | ||
| let mut cache = SourceCache::new(); | ||
| let parsed = parse_typescript_with_cache(&src, "test.ts", &mut cache) | ||
| .expect("parse should succeed"); | ||
| lower_module(&parsed.module, "test", "test.ts").expect("lower should succeed") | ||
| }) | ||
| .expect("spawn lower thread") | ||
| .join() | ||
| .expect("lower thread panicked") | ||
| } | ||
|
|
||
| fn find_local_type<'m>(module: &'m Module, name: &str) -> &'m Type { | ||
| for s in &module.init { | ||
| if let Stmt::Let { name: n, ty, .. } = s { | ||
| if n == name { | ||
| return ty; | ||
| } | ||
| } | ||
| } | ||
| panic!("binding {name} not found in module.init"); | ||
| } | ||
|
|
||
| #[test] | ||
| fn chained_fill_keeps_number_array_type() { | ||
| let module = lower_src("const a = new Array<number>(8192).fill(0);"); | ||
| assert_eq!( | ||
| find_local_type(&module, "a"), | ||
| &Type::Array(Box::new(Type::Number)), | ||
| "new Array<number>(n).fill(0) must infer number[], not Any" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn double_chain_through_receiver_returning_methods() { | ||
| let module = lower_src("const b = new Array<number>(4).fill(1).reverse();"); | ||
| assert_eq!( | ||
| find_local_type(&module, "b"), | ||
| &Type::Array(Box::new(Type::Number)), | ||
| "receiver-returning chains off new Array<T> must keep T[]" | ||
| ); | ||
| } | ||
|
|
||
| #[test] | ||
| fn element_returning_method_sees_element_type() { | ||
| let module = lower_src("const c = new Array<number>(4).fill(2).at(0);"); | ||
| assert_eq!( | ||
| find_local_type(&module, "c"), | ||
| &Type::Number, | ||
| ".at() on a new Array<number> chain must return the element type" | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Use
built-inin the release note.Change “builtin methods” to “built-in methods”.
🧰 Tools
🪛 LanguageTool
[grammar] ~1-~1: Ensure spelling is correct
Context: Kept the inferred array type through builtin methods chained on
new Array<T>(n)— ...(QB_NEW_EN_ORTHOGRAPHY_ERROR_IDS_1)
🤖 Prompt for AI Agents
Source: Linters/SAST tools