Skip to content
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.

Sign up for GitHub

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

fix: crash on assignment chain #2767

Merged
merged 2 commits into from
Oct 17, 2023
Merged

fix: crash on assignment chain #2767

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ecd7ad1
fix: crash on assignment chain
HerrCai0907 Oct 16, 2023
38821c1
fix lint
HerrCai0907 Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/compiler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5811,14 +5811,14 @@ export class Compiler extends DiagnosticEmitter {
assert(getterInstance.signature.thisType == thisType);
let returnType = getterInstance.signature.returnType;
let returnTypeRef = returnType.toRef();
let tempThis = flow.getTempLocal(returnType);
let tempThis = flow.getTempLocal(thisType);
let ret = module.block(null, [
this.makeCallDirect(setterInstance, [
module.local_tee(tempThis.index, thisExpr, returnType.isManaged),
module.local_tee(tempThis.index, thisExpr, /* isManaged=*/false, thisType.toRef()), // thisType is managed but here it must be alive
valueExpr
], valueExpression),
this.makeCallDirect(getterInstance, [
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be calling the getter here? I tried this in the Node.js REPL:

> let x = { get y() { console.log("get"); return 123 }, set y(z) { console.log("set", z) } }
undefined
> let foo = x.y = 456
set 456
undefined
> foo
456

It doesn't seem to call the getter, unless I'm missing something...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I do realize you didn't write this, but I wanted to point out what I noticed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we can use temp variable to store it?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, valueExpr should be tee'd to a temp local in the setter call, and the getter call should be replaced with a local.get to that local, if I'm correct about not calling that getter.

module.local_get(tempThis.index, returnTypeRef)
module.local_get(tempThis.index, thisType.toRef())
], valueExpression)
], returnTypeRef);
return ret;
Expand Down
98 changes: 98 additions & 0 deletions tests/compiler/assignment-chain.debug.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,98 @@
(module
(type $i32_i64_=>_none (func (param i32 i64)))
(type $i32_=>_none (func (param i32)))
(type $i32_=>_i64 (func (param i32) (result i64)))
(type $i32_i32_i32_i32_=>_none (func (param i32 i32 i32 i32)))
(type $none_=>_none (func))
(import "env" "abort" (func $~lib/builtins/abort (param i32 i32 i32 i32)))
(global $~lib/memory/__data_end i32 (i32.const 8))
(global $~lib/memory/__stack_pointer (mut i32) (i32.const 32776))
(global $~lib/memory/__heap_base i32 (i32.const 32776))
(memory $0 0)
(table $0 1 1 funcref)
(elem $0 (i32.const 1))
(export "memory" (memory $0))
(export "foo" (func $export:assignment-chain/foo))
(func $assignment-chain/A#set:y (param $this i32) (param $y i64)
local.get $this
local.get $y
i64.store $0 offset=8
)
(func $assignment-chain/A#get:y (param $this i32) (result i64)
local.get $this
i64.load $0 offset=8
)
(func $assignment-chain/A#set:x (param $this i32) (param $x i64)
local.get $this
local.get $x
i64.store $0
)
(func $~stack_check
global.get $~lib/memory/__stack_pointer
global.get $~lib/memory/__data_end
i32.lt_s
if
i32.const 32800
i32.const 32848
i32.const 1
i32.const 1
call $~lib/builtins/abort
unreachable
end
)
(func $assignment-chain/foo (param $a i32)
(local $1 i32)
(local $2 i32)
global.get $~lib/memory/__stack_pointer
i32.const 8
i32.sub
global.set $~lib/memory/__stack_pointer
call $~stack_check
global.get $~lib/memory/__stack_pointer
i64.const 0
i64.store $0
local.get $a
local.set $2
global.get $~lib/memory/__stack_pointer
local.get $2
i32.store $0
local.get $2
local.get $a
local.tee $1
local.set $2
global.get $~lib/memory/__stack_pointer
local.get $2
i32.store $0 offset=4
local.get $2
i64.const 1
call $assignment-chain/A#set:y
local.get $1
local.set $2
global.get $~lib/memory/__stack_pointer
local.get $2
i32.store $0 offset=4
local.get $2
call $assignment-chain/A#get:y
call $assignment-chain/A#set:x
global.get $~lib/memory/__stack_pointer
i32.const 8
i32.add
global.set $~lib/memory/__stack_pointer
)
(func $export:assignment-chain/foo (param $0 i32)
global.get $~lib/memory/__stack_pointer
i32.const 4
i32.sub
global.set $~lib/memory/__stack_pointer
call $~stack_check
global.get $~lib/memory/__stack_pointer
local.get $0
i32.store $0
local.get $0
call $assignment-chain/foo
global.get $~lib/memory/__stack_pointer
i32.const 4
i32.add
global.set $~lib/memory/__stack_pointer
)
)
4 changes: 4 additions & 0 deletions tests/compiler/assignment-chain.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"asc_flags": [
]
}
69 changes: 69 additions & 0 deletions tests/compiler/assignment-chain.release.wat
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
(module
(type $i32_i32_i32_i32_=>_none (func (param i32 i32 i32 i32)))
(type $i32_=>_none (func (param i32)))
(import "env" "abort" (func $~lib/builtins/abort (param i32 i32 i32 i32)))
(global $~lib/memory/__stack_pointer (mut i32) (i32.const 33792))
(memory $0 0)
(export "memory" (memory $0))
(export "foo" (func $export:assignment-chain/foo))
(func $export:assignment-chain/foo (param $0 i32)
(local $1 i32)
global.get $~lib/memory/__stack_pointer
i32.const 4
i32.sub
global.set $~lib/memory/__stack_pointer
block $folding-inner0
global.get $~lib/memory/__stack_pointer
i32.const 1024
i32.lt_s
br_if $folding-inner0
global.get $~lib/memory/__stack_pointer
local.tee $1
local.get $0
i32.store $0
local.get $1
i32.const 8
i32.sub
global.set $~lib/memory/__stack_pointer
global.get $~lib/memory/__stack_pointer
i32.const 1024
i32.lt_s
br_if $folding-inner0
global.get $~lib/memory/__stack_pointer
local.tee $1
i64.const 0
i64.store $0
local.get $1
local.get $0
i32.store $0
local.get $1
local.get $0
i32.store $0 offset=4
local.get $0
i64.const 1
i64.store $0 offset=8
local.get $1
local.get $0
i32.store $0 offset=4
local.get $0
local.get $0
i64.load $0 offset=8
i64.store $0
local.get $1
i32.const 8
i32.add
global.set $~lib/memory/__stack_pointer
global.get $~lib/memory/__stack_pointer
i32.const 4
i32.add
global.set $~lib/memory/__stack_pointer
return
end
i32.const 33824
i32.const 33872
i32.const 1
i32.const 1
call $~lib/builtins/abort
unreachable
)
)
8 changes: 8 additions & 0 deletions tests/compiler/assignment-chain.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
class A {
x: i64 = 0;
y: i64 = 0;
}

export function foo(a: A): void {
a.x = a.y = 1;
}