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

Jump to bottom

Implicity is ignored #919

Closed
chabad360 opened this issue Oct 22, 2020 · 2 comments · Fixed by #974
Closed

Implicity is ignored #919

chabad360 opened this issue Oct 22, 2020 · 2 comments · Fixed by #974
Assignees
@mpl
Labels
area/core bug Something isn't working
Milestone
v0.13.x

Comments

@chabad360
Copy link

The following program sample.go triggers a panic:

package main

import (
	"fmt"
	"io"
	"reflect"
)

type TestStruct struct{}

func (t TestStruct) Write(p []byte) (n int, err error) {
	fmt.Println(len(p))
	return len(p), nil
}

func TestFunc() {
	aType := reflect.TypeOf((*io.Writer)(nil)).Elem()
	bType := reflect.TypeOf(TestStruct{})

	fmt.Println(bType.Implements(aType))
}

func main() {
	TestFunc()
	var t interface{}
	t = TestStruct{}
	t.(io.Writer).Write(nil)
}

Expected result:

$ go run ./sample.go
true
0

Got:

$ yaegi ./sample.go
false
./sample.go:24:2: panic
run: reflect: call of reflect.Value.Elem on struct Value
goroutine 1 [running]:
runtime/debug.Stack(0x1, 0xc000300600, 0x40)
        /usr/lib/go/src/runtime/debug/stack.go:24 +0x9f
github.com/traefik/yaegi/interp.(*Interpreter).eval.func1(0xc00013dc70)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/interp.go:455 +0xc5
panic(0xe01ac0, 0xc000117d00)
        /usr/lib/go/src/runtime/panic.go:969 +0x1b9
github.com/traefik/yaegi/interp.runCfg.func1(0xc0003b20a0, 0xc000487f00)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/run.go:124 +0x217
panic(0xe01ac0, 0xc000117d00)
        /usr/lib/go/src/runtime/panic.go:969 +0x1b9
reflect.Value.Elem(0xe4f980, 0xc000117cc0, 0x199, 0x199, 0x0, 0x0)
        /usr/lib/go/src/reflect/value.go:842 +0x1a5
github.com/traefik/yaegi/interp.typeAssert.func2(0xc0003b20a0, 0xc000117960)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/run.go:197 +0x78
github.com/traefik/yaegi/interp.runCfg(0xc000487f00, 0xc0003b20a0)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/run.go:130 +0x6e
github.com/traefik/yaegi/interp.(*Interpreter).run(0xc00037e000, 0xc000487800, 0xc0003b2000)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/run.go:108 +0x2af
github.com/traefik/yaegi/interp.(*Interpreter).eval(0xc00037e000, 0xc0002fe540, 0x1a2, 0x7ffdcb95f6c6, 0xa, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/interp.go:547 +0x809
github.com/traefik/yaegi/interp.(*Interpreter).EvalPath(0xc00037e000, 0x7ffdcb95f6c6, 0xa, 0xc0002fe300, 0x1a2, 0x0, 0x0, 0xc000125520)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/interp/interp.go:394 +0xfa
main.runFile(0xc00037e000, 0x7ffdcb95f6c6, 0xa, 0xc00010e3c0, 0x1)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/cmd/yaegi/run.go:123 +0xae
main.run(0xc00000e090, 0x1, 0x1, 0x13, 0x13)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/cmd/yaegi/run.go:89 +0x7e5
main.main()
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.3/cmd/yaegi/yaegi.go:144 +0x415
@mvertes mvertes added area/core bug Something isn't working labels Oct 23, 2020
@mvertes mvertes added this to the v0.9.x milestone Oct 23, 2020
@chabad360
Copy link
Author

As of 0.9.5:

$ yaegi ./sample.go
false
./sample.go:27:2: panic
run: reflect: call of reflect.Value.Elem on struct Value
goroutine 1 [running]:
runtime/debug.Stack(0x1, 0xc000188a00, 0x40)
        /usr/lib/go/src/runtime/debug/stack.go:24 +0x9f
github.com/traefik/yaegi/interp.(*Interpreter).eval.func1(0xc000239c70)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/interp.go:480 +0xc5
panic(0xe02c00, 0xc000482ba0)
        /usr/lib/go/src/runtime/panic.go:969 +0x1b9
github.com/traefik/yaegi/interp.runCfg.func1(0xc0001d0820, 0xc000488300, 0xc000239a60)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/run.go:171 +0x247
panic(0xe02c00, 0xc000482ba0)
        /usr/lib/go/src/runtime/panic.go:969 +0x1b9
reflect.Value.Elem(0xe50d20, 0xc000482b60, 0x199, 0x199, 0x0, 0x0)
        /usr/lib/go/src/reflect/value.go:842 +0x1a5
github.com/traefik/yaegi/interp.typeAssert.func2(0xc0001d0820, 0xc000482800)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/run.go:244 +0x78
github.com/traefik/yaegi/interp.runCfg(0xc000488300, 0xc0001d0820)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/run.go:177 +0x87
github.com/traefik/yaegi/interp.(*Interpreter).run(0xc0003ae000, 0xc000487c00, 0xc0001d0780)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/run.go:108 +0x2af
github.com/traefik/yaegi/interp.(*Interpreter).eval(0xc0003ae000, 0xc000176700, 0x1a1, 0x7ffcfa41a6c7, 0x8, 0x0, 0x0, 0x0, 0x0, 0x0, ...)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/interp.go:572 +0x809
github.com/traefik/yaegi/interp.(*Interpreter).EvalPath(0xc0003ae000, 0x7ffcfa41a6c7, 0x8, 0xc000176500, 0x1a1, 0x0, 0x0, 0xc0003f60d0)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/interp/interp.go:394 +0xfa
main.runFile(0xc0003ae000, 0x7ffcfa41a6c7, 0x8, 0xc000138cc0, 0x1)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/cmd/yaegi/run.go:123 +0xae
main.run(0xc000128030, 0x1, 0x1, 0x13, 0x13)
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/cmd/yaegi/run.go:89 +0x7e5
main.main()
        /home/mendel/go/pkg/mod/github.com/traefik/yaegi@v0.9.5/cmd/yaegi/yaegi.go:144 +0x415

@mpl mpl self-assigned this Nov 5, 2020
@mpl mpl mentioned this issue Nov 10, 2020
Closed
@chabad360 chabad360 mentioned this issue Nov 25, 2020
Open
@mpl mpl mentioned this issue Nov 26, 2020
Closed
mpl added a commit to mpl/yaegi that referenced this issue Dec 1, 2020
@mpl
interp: fix short-form type assertions
0589a41 
The long-form (with comma-ok) ones were already fixed but the short-form
ones were not because they were in a completely different code path.

This PR also refactors the code so that both short-form and long-form
are now merged in the same function.

N.B: even though most (all?) cases seem to now be supported, one of them
still yields a result that does not satisfy reflect's Implements method
yet. It does not prevent the resulting assertion to be usable though.

N.B2: the code path for the third-form (_, ok) hasn't been fixed and/or
refactored yet.

Fixes traefik#919
@mpl mpl mentioned this issue Dec 1, 2020
Merged
traefiker pushed a commit that referenced this issue Dec 2, 2020
@mpl
interp: fix short-form type assertions
2db4579 
The long-form (with comma-ok) ones were already fixed but the short-form
ones were not because they were in a completely different code path.

This PR also refactors the code so that both short-form and long-form
are now merged in the same function.

N.B: even though most (all?) cases seem to now be supported, one of them
still yields a result that does not satisfy reflect's Implements method
yet. It does not prevent the resulting assertion to be usable though.

N.B2: the code path for the third-form (_, ok) hasn't been fixed and/or
refactored yet.

Fixes #919
@chabad360
Copy link
Author

It should be noted that the reflect.Implements call is still not working. Should another issue be opened for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@mpl mpl

Labels
area/core bug Something isn't working
Projects
None yet
Milestone
v0.13.x
Development

Successfully merging a pull request may close this issue.

interp: fix short-form type assertions mpl/yaegi
3 participants
@mpl @chabad360 @mvertes