Repository navigation
js_parser: declare decorator lowering temporaries per evaluation in parameter defaults and field initializers #38904
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
base: main
Are you sure you want to change the base?
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 |
|---|---|---|
|
|
@@ -604,6 +604,12 @@ pub struct P<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> { | |
| /// Name from assignment context for anonymous decorated class expressions. | ||
| /// Set before visitExpr, consumed by lowerStandardDecoratorsImpl. | ||
| pub(crate) decorator_class_name: Option<&'a [u8]>, | ||
| /// Set while visiting an expression that runs once per call or per | ||
| /// construction (a parameter default value, an instance field initializer) | ||
| /// although `nearest_stmt_list` runs once: a declaration hoisted there would | ||
| /// be shared by every evaluation of the expression. `visit_stmts` clears it | ||
| /// for the statements of any body nested inside such an expression. | ||
|
Comment on lines
+607
to
+611
Contributor
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. If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code |
||
| pub(crate) nearest_stmt_list_is_shared: bool, | ||
| } | ||
|
|
||
| // `binding::ToExprWrapper` type-erases `*P` (which is generic over | ||
|
|
@@ -8847,6 +8853,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| will_wrap_module_in_try_catch_for_using: false, | ||
| nearest_stmt_list: None, | ||
| decorator_class_name: None, | ||
| nearest_stmt_list_is_shared: false, | ||
|
|
||
| jsx_transform, | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -218,6 +218,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| duplicate_args_check = Some(StringVoidMap::get()); | ||
| } | ||
|
|
||
| // Default values (including those nested in destructuring patterns) run | ||
| // once per call, while the statement list enclosing the function runs | ||
| // once; the function's own body installs its list later. | ||
|
Comment on lines
+221
to
+223
Contributor
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. If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code |
||
| let old_nearest_stmt_list_is_shared = self.nearest_stmt_list_is_shared; | ||
| self.nearest_stmt_list_is_shared = true; | ||
|
|
||
| for arg in args.iter_mut() { | ||
| if arg.ts_decorators.len_u32() > 0 { | ||
| self.visit_ts_decorators(&mut arg.ts_decorators); | ||
|
|
@@ -230,6 +236,8 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| self.visit_expr(default); | ||
| } | ||
| } | ||
|
|
||
| self.nearest_stmt_list_is_shared = old_nearest_stmt_list_is_shared; | ||
| } | ||
|
|
||
| // `Vec<Expr>` is not `Copy`; mutate in place. | ||
|
|
@@ -984,7 +992,12 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| } | ||
|
|
||
| if let Some(val) = property.initializer { | ||
| // if (property.flags.is_static and ) | ||
| // An instance initializer runs once per construction; a | ||
| // static one runs with the class itself. | ||
|
Comment on lines
+995
to
+996
Contributor
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. If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code |
||
| let old_nearest_stmt_list_is_shared = self.nearest_stmt_list_is_shared; | ||
| if !property.flags.contains(flags::Property::IsStatic) { | ||
| self.nearest_stmt_list_is_shared = true; | ||
| } | ||
| if let Some(name) = name_to_keep { | ||
| let was_anon = val.is_anonymous_named(); | ||
| let prev_dcn2 = self.decorator_class_name; | ||
|
|
@@ -1003,6 +1016,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| } else { | ||
| self.visit_expr(property.initializer.as_mut().unwrap()); | ||
| } | ||
| self.nearest_stmt_list_is_shared = old_nearest_stmt_list_is_shared; | ||
| } | ||
|
|
||
| // manual restore for the three `defer`s above | ||
|
|
@@ -1341,11 +1355,14 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
| ListManaged::with_capacity_in(stmts.len(), p.arena); | ||
|
|
||
| let prev_nearest_stmt_list = p.nearest_stmt_list; | ||
| let prev_nearest_stmt_list_is_shared = p.nearest_stmt_list_is_shared; | ||
| // BACKREF — `before` outlives this block; raw NonNull avoids | ||
| // the `&'a mut` borrow conflict. Derive via `addr_of_mut!` (no intermediate | ||
| // `&mut`) so the pointer shares the local's base tag and survives the | ||
| // direct `&mut before` reborrows in the loop below (Stacked Borrows). | ||
| p.nearest_stmt_list = NonNull::new(core::ptr::addr_of_mut!(before)); | ||
| // `before` runs every time the statements it is emitted ahead of run. | ||
| p.nearest_stmt_list_is_shared = false; | ||
|
|
||
| let mut preprocessed_enum_i: usize = 0; | ||
|
|
||
|
|
@@ -1538,6 +1555,7 @@ impl<'a, const TYPESCRIPT: bool, const SCAN_ONLY: bool> P<'a, TYPESCRIPT, SCAN_O | |
|
|
||
| // manual restore for the block-level `defer`s | ||
| p.nearest_stmt_list = prev_nearest_stmt_list; | ||
| p.nearest_stmt_list_is_shared = prev_nearest_stmt_list_is_shared; | ||
| p.is_control_flow_dead = old_is_control_flow_dead; | ||
| } | ||
|
|
||
|
|
||
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.
If you need a paragraph-long comment to justify why the workaround is OK, the code is wrong — fix the code