Skip to content

Commit

Permalink
fix(compiler): closure transform generated parent_this even when no…
Browse files Browse the repository at this point in the history
…t used (#6531)

This is a small code cleanup as part of addressing #4925.

We only generate the `parent_this` variable used in generated closure classes when needed. If the inflight closure never accesses `this` then we don't generated `parent_this`. The code avoids traversing into inner classes inside the closure when searching for `this` accesses, making it a bit cleaner and more efficient.

This relates to #4925 because it solves the problem of accessing `this` (for assignment into `parent_this`) before the `super()` call in a ctor.

## Checklist

- [x] Title matches [Winglang's style guide](https://www.winglang.io/contributing/start-here/pull_requests#how-are-pull-request-titles-formatted)
- [x] Description explains motivation and solution
- [ ] Tests added (always)
- [ ] Docs updated (only required for features)
- [ ] Added `pr/e2e-full` label if this feature requires end-to-end testing

*By submitting this pull request, I confirm that my contribution is made under the terms of the [Wing Cloud Contribution License](https://github.com/winglang/wing/blob/main/CONTRIBUTION_LICENSE.md)*.
  • Loading branch information
yoav-steinberg committed May 22, 2024
1 parent aa78e00 commit 3d2aaa2
Show file tree
Hide file tree
Showing 11 changed files with 14 additions and 34 deletions.
38 changes: 14 additions & 24 deletions libs/wingc/src/closure_transform.rs
Original file line number Diff line number Diff line change
Expand Up @@ -156,16 +156,12 @@ impl Fold for ClosureTransformer {
let class_fields: Vec<ClassField> = vec![];
let class_init_params: Vec<FunctionParameter> = vec![];

let parent_this = format!("{}_{}", PARENT_THIS_NAME, self.closure_counter);
let mut this_transform = RenameThisTransformer::new(&parent_this.as_str());
let new_func_def = if self.inside_scope_with_this {
// If we are inside a class, we transform inflight closures with an extra
// `let __parent_this_${CLOSURE_COUNT} = this;` statement before the class definition, and replace references
// to `this` with `__parent_this_${CLOSURE_COUNT}` so that they can access the parent class's fields.
let parent_this = format!("{}_{}", PARENT_THIS_NAME, self.closure_counter);
let mut this_transform = RenameThisTransformer {
from: "this",
to: parent_this.as_str(),
inside_class: false,
};
this_transform.fold_function_definition(func_def)
} else {
func_def
Expand Down Expand Up @@ -226,11 +222,8 @@ impl Fold for ClosureTransformer {

// If we are inside a scope with "this", add define `let __parent_this_${CLOSURE_COUNT} = this` which can be
// used by the newly-created preflight classes
if self.inside_scope_with_this {
let parent_this_name = Symbol::new(
format!("{}_{}", PARENT_THIS_NAME, self.closure_counter),
WingSpan::for_file(file_id),
);
if self.inside_scope_with_this && this_transform.performed_renames {
let parent_this_name = Symbol::new(parent_this, WingSpan::for_file(file_id));
let this_name = Symbol::new("this", WingSpan::for_file(file_id));
let parent_this_def = Stmt {
kind: StmtKind::Let {
Expand Down Expand Up @@ -331,34 +324,31 @@ impl Fold for ClosureTransformer {
struct RenameThisTransformer<'a> {
from: &'a str,
to: &'a str,
// The transform shouldn't change references to `this` inside inflight classes since
// they refer to different objects.
inside_class: bool,
performed_renames: bool,
}

impl Default for RenameThisTransformer<'_> {
fn default() -> Self {
impl<'a> RenameThisTransformer<'a> {
fn new(to: &'a str) -> Self {
Self {
from: "this",
to: PARENT_THIS_NAME,
inside_class: false,
to,
performed_renames: false,
}
}
}

impl<'a> Fold for RenameThisTransformer<'a> {
fn fold_class(&mut self, node: Class) -> Class {
let old_inside_class = self.inside_class;
self.inside_class = true;
let new_class = fold::fold_class(self, node);
self.inside_class = old_inside_class;
new_class
// The transform shouldn't change references to `this` inside inflight classes since
// they refer to different objects. Skip inner class.
node
}

fn fold_reference(&mut self, node: Reference) -> Reference {
match node {
Reference::Identifier(ident) => {
if !self.inside_class && ident.name == self.from {
if ident.name == self.from {
self.performed_renames = true;
Reference::Identifier(Symbol {
name: self.to.to_string(),
span: ident.span,
Expand Down
1 change: 0 additions & 1 deletion libs/wingc/src/jsify/snapshots/closure_field.snap
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,6 @@ class $Root extends $stdlib.std.Resource {
class MyResource extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
const __parent_this_1 = this;
class $Closure1 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,6 @@ class $Root extends $stdlib.std.Resource {
}
defineBucket(name) {
const b = this.node.root.new("@winglang/sdk.cloud.Bucket", cloud.Bucket, this, name);
const __parent_this_1 = this;
class $Closure1 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,6 @@ class $Root extends $stdlib.std.Resource {
class Foo extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
const __parent_this_1 = this;
class $Closure1 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -163,7 +163,6 @@ class $Root extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
this.bucket = this.node.root.new("@winglang/sdk.cloud.Bucket", cloud.Bucket, this, "Bucket");
const __parent_this_1 = this;
class $Closure1 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,7 +328,6 @@ class $Root extends $stdlib.std.Resource {
super($scope, $id);
}
makeFunc(handler) {
const __parent_this_2 = this;
class $Closure2 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,6 @@ class $Root extends $stdlib.std.Resource {
});
}
}
const __parent_this_2 = this;
class $Closure2 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ class $Root extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
const foo = (() => {
const __parent_this_1 = this;
class $Closure1 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -388,7 +388,6 @@ class $Root extends $stdlib.std.Resource {
class MyResource extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
const __parent_this_4 = this;
class $Closure4 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,6 @@ class $Root extends $stdlib.std.Resource {
constructor($scope, $id, ) {
super($scope, $id);
this.data = this.node.root.new("@winglang/sdk.cloud.Bucket", cloud.Bucket, this, "Bucket");
const __parent_this_1 = this;
class $Closure1 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ class $Root extends $stdlib.std.Resource {
super($scope, $id);
const s = "inResource";
$helpers.assert($helpers.eq(s, "inResource"), "s == \"inResource\"");
const __parent_this_2 = this;
class $Closure2 extends $stdlib.std.AutoIdResource {
_id = $stdlib.core.closureId();
constructor($scope, $id, ) {
Expand Down

0 comments on commit 3d2aaa2

Please sign in to comment.