Skip to content

Struct values not being cloned when assigned from/called with when variable type is interface #661

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

Open
myitcv opened this issue Jul 26, 2017 · 2 comments · May be fixed by #669
Open

Struct values not being cloned when assigned from/called with when variable type is interface #661

myitcv opened this issue Jul 26, 2017 · 2 comments · May be fixed by #669

Comments

@myitcv
Copy link
Member

myitcv commented Jul 26, 2017

(had a look, couldn't recall this having been discussed or raised before)

Consider the following code:

package main

import (
	"fmt"
)

type S struct {
	Name string
}

func (s S) SetName(n string) {
	s.Name = n
}

type NameSet interface {
	SetName(n string)
}

func main() {
	s1 := S{}
	var s2 NameSet = s1
	s1.Name = "Rob"
	
	fmt.Printf("s1: %v, s2: %v\n", s1, s2)
	
	s2.SetName("Pike")
	fmt.Printf("s1: %v, s2: %v\n", s1, s2)
}

Per the Go Playground, this should output:

s1: {Rob}, s2: {}
s1: {Rob}, s2: {}

But per the GopherJS Playground we get:

s1: {Rob}, s2: {Rob} 
s1: {Pike}, s2: {Pike} 

Reason being the assignment to s2 is not $clone-ing the value from s1.

Similarly, in the call s2.SetName we are not $clone-ing.

If the type of s2 is changed to S then everything works as expected.

Will look into this a bit more later, just logging here for now.

@dmitshur
Copy link
Member

dmitshur commented Jul 26, 2017

Thanks for reporting, that looks like a legitimate bug related to JavaScript references vs Go values/pointers. /cc @neelance

@myitcv
Copy link
Member Author

myitcv commented Jul 31, 2017

I've pushed up #669 as an attempt to fix this bug.

@neelance @shurcooL - would very much appreciate feedback and comments.

myitcv added a commit to myitcv/gopherjs that referenced this issue Aug 3, 2017
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc.  Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the source
variable are, incorrectly, visible via the target variable.
gopherjs#661 gives a simple example.
There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Aug 3, 2017
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

For some reason the changes that address the aforementioned issues start
to tip the std library tests over the V8 stack size limit, causing
text/template TestMaxExecDepth to fail randomly locally and on CI. This
had previously been addressed by @neelance in
1f89545; the --stack_size flag was
passed to NodeJS which in turn passed the value onto V8. But per
nodejs/node#14567 (comment) it
was pointed out that the value of ulimit -s must be >= the value of
--stack_size for the --stack_size to make any sort of sense. Hence this
commit also harmonises the setting of ulimit -s during the CI test run
with the value of --stack_size that is passed to NodeJS (which in turn
passes that value onto V8) when running either gopherjs test or gopherjs
run.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Aug 27, 2017
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Aug 27, 2017
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Apr 1, 2018
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Apr 1, 2018
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Apr 14, 2018
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Apr 16, 2018
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Apr 20, 2018
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
myitcv added a commit to myitcv/gopherjs that referenced this issue Apr 21, 2018
As outlined in gopherjs#661, the
compiler fails to generate code to copy a struct/array value for an
assignment when the target's underlying type is an interface type,
whether for explicit variable assignments, implicit function/method
parameters etc. Instead, taking the example of explicit variable
assignment, the interface variable is assigned a value that contains the
same pointer to the source struct/array val (we're in Javascript world,
so everything is a pointer). This means that changes to the struct/array
value via the source variable are, incorrectly, visible via the target
variable. gopherjs#661 gives a simple
example. There is a further issue when interface values are assigned to
interface-typed variables: struct/array values are not copied when they
should be.

Fixes gopherjs#661.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants