Instance defined with VmModules is not a new instance

[houxul]

Anko code

module User{
	_age = -1
	_name = "undefined"

	func SetAge(age) {
		_age = age
	}

	func SetName(name) {
		_name = name
	}
}

data = [
	{
		"age": 10,
		"name" : "jack"
	},
	{
		"age": 0,
		"name" : ""
	},
]

for item in data {
	user = User
	if item["age"] != 0 {
		user.SetAge(item["age"])
	}

	if item["name"] != "" {
		user.SetName(item["name"])
	}
	
	println(user._age)
	println(user._name)
}

Expected output

10
jack
-1
undefined

Actual output

10
jack
10
jack

[MichaelS11]

Looks like a bug to me. Another example:

package main

import (
	"fmt"
	"log"

	"github.com/mattn/anko/vm"
)

func main() {
	env := vm.NewEnv()

	err := env.Define("println", fmt.Println)
	if err != nil {
		log.Fatalf("Define error: %v\n", err)
	}

	script := `
module rectangle {
	_length = 1
	_width = 1
	
	func setLength (length) {
		if length <= 0 {
			return
		}
		_length = length
	}
	
	func setWidth (width) {
		if width <= 0 {
			return
		}
		_width = width
	}
	
	func area () {
		return _length * _width
	}
	
	func perimeter () {
		return 2 * (_length + _width)
	}
 }

var rectangle1 = rectangle

rectangle1.setLength(4)
rectangle1.setWidth(5)

var rectangle2 = rectangle

rectangle2.setLength(2)
rectangle2.setWidth(4)

println(rectangle1.area())
println(rectangle1.perimeter())

println(rectangle2.area())
println(rectangle2.perimeter())
`

	_, err = env.Execute(script)
	if err != nil {
		log.Fatalf("Execute error: %v\n", err)
	}
}

Output:

8
12
8
12

Expected output:

20
18
8
12

[MichaelS11]

@mattn

So for var rectangle1 = rectangle the right side is an IdentExpr, which means it is basically calling env.get("rectangle"). This is setting rectangle1 to an *Env which is the same *Env that rectangle points to. Here is the line of code that is doing that setting: https://github.com/mattn/anko/blob/adf6aeefec2c5f68e58c142e1e5ae3be250289e9/vm/env.go#L277

I know this can be fixed by creatinga copy of the env, but not sure if that is the correct approach or where that code should go. Any thoughts @mattn ?

[mattn]

I don't look code yet but it seems correct behavior.

[MichaelS11]

@mattn

I think there would be a difference between rectangle1 = rectangle and var rectangle1 = rectangle but they do the same thing, copy the pointer instead of copying the whole module.

[traetox]

Is there a work around that would allow for instantiating a new module?

Module "types" don't work with new or make.

[mattn]

Most of users not use module. So we can change specification of module, I think.

[MichaelS11]

I am going to look into this more, see if there is something not too hard to make this work.

[traetox]

Even if we could figure out how to hook the new() function to allow for copying that environment, that would be cool too. I don't want to advocate for changing the language spec if at all possible.

[MichaelS11]

The new spec is looking for a type, so would like to look at other options.

So having a module named a, like: module a { b = 1 } Here are some thoughts for a copy of a to c:

1. module c = a
2. var c = a
3. c := a

Please let me know if you have any other ideas and your thoughts about the above.

[traetox]

The var keyword being used to declare a new variable seems to mesh with the way normal types work. So thats my vote (if I get one). :)

On Tue, Dec 3, 2019, 5:12 PM MichaelS11 [email protected] wrote:

The new spec is looking for a type, so would like to look at other options.

So having a module named a, like: module a { b = 1 } Here are some thoughts for a copy of a to c:

1. module c = a
2. var c = a
3. c := a

Please let me know if you have any other ideas and your thoughts about the above.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/mattn/anko/issues/315?email_source=notifications&email_token=AAKD2ODFIO537PB57CA5R5TQW3YYVA5CNFSM4IFXFXW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF3IL3A#issuecomment-561415660, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKD2OGEZ4PRGE77VCYXXATQW3YYVANCNFSM4IFXFXWQ .

[MichaelS11]

@mattn What you think?

[mattn]

On my first design, module meant namespace not class. So I did not expect that module can be instance with new keyword. But most of users does not use module and they seems to expect classes. We may have to add classes.

[MichaelS11]

So rename module to class? Have some details on what you would like?

[mattn]

Let's add new code for class and make "module" deprecated.

Go's reflect does not have APIs to make struct dynamically. So adding class on anko will be useful.

Class has methods and self-keyword, and definition of variables.

[MichaelS11]

Yes it does. https://golang.org/pkg/reflect/#StructOf

Sounds like we just need to support struct better and that should work instead of classes.

[mattn]

Yes, few years ago, I worked in progress for struct. But some APIs did not exists in that time.

https://github.com/mattn/anko/commit/158d91ccdd07c67e99204497290ec9e687b7a0cd

[MichaelS11]

Ok, so deprecate modules and add better support for structs.

[MichaelS11]

@mattn What about this PR https://github.com/mattn/anko/pull/322 for now?

[MichaelS11]

@traetox Please test

[traetox]

Will do. Thank you!

On Wed, Dec 4, 2019, 7:05 PM MichaelS11 [email protected] wrote:

@traetox https://github.com/traetox Please test

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/mattn/anko/issues/315?email_source=notifications&email_token=AAKD2OFGNN7HB2PSARSFZ6DQXBOVFA5CNFSM4IFXFXW2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEF7GT6Q#issuecomment-561932794, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAKD2OC6TDJI2XQQ3U67AW3QXBOVFANCNFSM4IFXFXWQ .

[MichaelS11]

@traetox All good?

[traetox]

doesn't look like the PR fixed it.

Here is a test script:

#!anko

module Foo {
  var x = "testing"
  func bar1() {
    return x
  }
  func bar2(z) {
     x = z
  }
}

var t1 = Foo
var t2 = Foo
t1.bar2("A")
t2.bar2("B")
println(t1.bar1())
println(t2.bar1())
println(Foo.bar1())

Using anko version 0.15 i got the following output

B
B
B

Expected output:

A
B
testing

[MichaelS11]

So t1 and t2 are new instances of Foo however the functions still point to the orginal variable x. I will think about it some more but I don't think there are any easy fixes for modules.

Will probably need to work on better struct support so can do Go struct methods. In partiluar, need to add struct creation.

Note that struct methods are supported to some extent, can look at Sort for an example: https://godoc.org/github.com/mattn/anko/vm#example-package--VmSort

So for now if you define the struct using env define, it might do what you are looking for.

[traetox]

ok, that seems entirely fair. I would say you could close this as "working as intended"

Thank you for your time! We will poke a bit as we can on the code base and see if we can get some PRs that make sense.

Thank you!