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

panic: interface conversion: ast.Expr is *ast.IndexExpr, not *ast.Ident #62

Closed
ainar-g opened this issue May 17, 2022 · 3 comments · Fixed by #64
Closed

panic: interface conversion: ast.Expr is *ast.IndexExpr, not *ast.Ident #62

ainar-g opened this issue May 17, 2022 · 3 comments · Fixed by #64

Comments

@ainar-g
Copy link
Contributor

ainar-g commented May 17, 2022

The generics support in golang.org/x/tools/go/ssa seems to be mostly ready (see golang/go#48525 (comment)), so I've updated it to the latest master commit, compiled unparam, and tried using it on some generic code. The result is a panic:

panic: interface conversion: ast.Expr is *ast.IndexExpr, not *ast.Ident

goroutine 1 [running]:
mvdan.cc/unparam/check.recvPrefix(...)
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check .go:880
mvdan.cc/unparam/check.(*Checker).declCounts(0xc000124960, {0xc0001c0c80, 0x2a}, {0xc0001c9fe8 , 0x6})
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check .go:855 +0x3f4
mvdan.cc/unparam/check.(*Checker).multipleImpls(0xc000124960, 0xc0001dbe00, 0xc002cc2000)
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check.go:893 +0x105
mvdan.cc/unparam/check.(*Checker).checkFunc(0xc000124960, 0xc002cc2000, 0xc0001c0c80?)
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check.go:493 +0x27e
mvdan.cc/unparam/check.(*Checker).Check(0xc000124960)
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check.go:377 +0x76b
mvdan.cc/unparam/check.(*Checker).lines(0xc000124960, {0xc000010190?, 0x4c736f?, 0x40c8c5?})
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check.go:107 +0x16a
mvdan.cc/unparam/check.UnusedParams(0x0, 0x0, 0x0, {0xc000010190, 0x2, 0x2})
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/check/check.go:44 +0xe5
main.main1()
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/main.go:37 +0x151
main.main()
        /home/ainar/go/pkg/mod/mvdan.cc/[email protected]/main.go:23 +0x19

The reproducer code:

package main

type set[T comparable] struct {
	m map[T]struct{}
}

func (s set[T]) has(v T) (ok bool) {
	_, ok = s.m[v]

	return ok
}

func main() {
	s := set[int]{}
	_ = s.has(0)
}

Versions:

go version -m ./bin/unparam
./bin/unparam: go1.18.2
        path    mvdan.cc/unparam
        mod     mvdan.cc/unparam        v0.0.0-20220316160445-06cc5682983b      h1:C8Pi6noat8BcrL9WnSRYeQ63fpkJk3hKVHtF5731kIw=
        dep     golang.org/x/mod        v0.6.0-dev.0.20220419223038-86c51ed26bb4        h1:6zppjxzCulZykYSLyVDYbneBfbaBIQPYMevg0bEwv2s=
        dep     golang.org/x/sys        v0.0.0-20220513210249-45d2b4557a2a      h1:N2T1jUrTQE9Re6TFF5PhvEHXHCguynGhKjWVsIUt5cY=
        dep     golang.org/x/tools      v0.1.11-0.20220517032152-814e0d74b53f   h1:uWV+yNYrw4eleKDoYWdMeHM4sAErndcO+Z71LxvDNvA=
        build   -compiler=gc
        build   CGO_ENABLED=1
        build   CGO_CFLAGS=
        build   CGO_CPPFLAGS=
        build   CGO_CXXFLAGS=
        build   CGO_LDFLAGS=
        build   GOARCH=amd64
        build   GOOS=linux
        build   GOAMD64=v1
@mvdan
Copy link
Owner

mvdan commented May 17, 2022

I haven't looked at supporting generics in unparam yet; my thinking was waiting until go/ssa fully supports them.

@ainar-g
Copy link
Contributor Author

ainar-g commented May 17, 2022

It's up to you, of course, but other SSA analysers, such as nilness, don't have issues with the code, so I assume that something useful could already be done. Especially considering the comment I've linked previously.

@ldez ldez mentioned this issue May 27, 2022
Closed
mvdan added a commit that referenced this issue Jun 29, 2022
@mvdan
add preliminary support for generic code
b9865a7 
Note that we don't yet support reporting unused parameters on generic
functions, as showed by the TODO in typeparams.txt, but at least we
don't panic as quickly as before.

Also note that running unparam on std currently panics as well,
presumably due to a bug in go/ssa that I will report shortly.

Thanks to Ainar Garipov for reporting and Ludovic Fernandez for
suggesting a fix.

Fixes #62.
@mvdan mvdan mentioned this issue Jun 29, 2022
Merged
mvdan added a commit that referenced this issue Jun 29, 2022
@mvdan
add preliminary support for generic code
cc9b2a7 
Note that we don't yet support reporting unused parameters on generic
functions, as showed by the TODO in typeparams.txt, but at least we
don't panic as quickly as before.

Also note that running unparam on std currently panics as well,
presumably due to a bug in go/ssa that I will report shortly.

Thanks to Ainar Garipov for reporting and Ludovic Fernandez for
suggesting a fix.

Fixes #62.
mvdan added a commit that referenced this issue Jun 29, 2022
@mvdan
add preliminary support for generic code
c18efef 
Note that we don't yet support reporting unused parameters on generic
functions, as showed by the TODO in typeparams.txt, but at least we
don't panic as quickly as before.

Also note that running unparam on std currently panics as well,
presumably due to a bug in go/ssa that I will report shortly.

Thanks to Ainar Garipov for reporting and Ludovic Fernandez for
suggesting a fix.

Note that we need to use x/exp/typeparams to keep support for Go 1.17.x.

Fixes #62.
mvdan added a commit that referenced this issue Jun 29, 2022
@mvdan
add preliminary support for generic code
c18a9b1 
Note that we don't yet support reporting unused parameters on generic
functions, as showed by the TODO in typeparams.txt, but at least we
don't panic as quickly as before.

Also note that running unparam on std currently panics as well,
presumably due to a bug in go/ssa that I will report shortly.

Thanks to Ainar Garipov for reporting and Ludovic Fernandez for
suggesting a fix.

Note that we need to use x/exp/typeparams to keep support for Go 1.17.x.

Fixes #62.
@mvdan mvdan closed this as completed in #64 Jul 6, 2022
mvdan added a commit that referenced this issue Jul 6, 2022
@mvdan
add preliminary support for generic code
678bad1 
Note that we don't yet support reporting unused parameters on generic
functions, as showed by the TODO in typeparams.txt, but at least we
don't panic as quickly as before.

Also note that running unparam on std currently panics as well,
presumably due to a bug in go/ssa that I will report shortly.

Thanks to Ainar Garipov for reporting and Ludovic Fernandez for
suggesting a fix.

Note that we need to use x/exp/typeparams to keep support for Go 1.17.x.

Fixes #62.
@ainar-g
Copy link
Contributor Author

ainar-g commented Jul 6, 2022

Pardon for not reviewing in time. I don't have the original code that triggered the issue by me right now, but running it on other generic code doesn't panic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
2 participants
@mvdan @ainar-g