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.

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

Issues with rangefunc in 1.23rc1 projects #1569

Closed
matta opened this issue Jun 29, 2024 · 4 comments
Closed

Issues with rangefunc in 1.23rc1 projects #1569

matta opened this issue Jun 29, 2024 · 4 comments
Labels

Comments

@matta
Copy link

matta commented Jun 29, 2024

Basic issue

  1. statichceck built at HEAD complains about rangefunc usage within the standard library.
  2. With GOEXPERIMENT=rangefunc set, staticcheck panics.

The second issue is a problem because currently gopls v0.16 says it supports rangefunc, but appears to require GOEXPERIMENT=rangefunc in 1.23rc1 projects before it stops reporting errors for them. See golang/go#68248

Steps and more information:

Create a project with go.mod

module example.com/m

go 1.23rc1

and main.go:

package main

import (
	"fmt"
	"maps"
)

func main() {
	m := make(map[string]int)
	m["a"] = 1
	m["b"] = 2

	for k, v := range maps.All(m) {
		fmt.Println(k, v)
	}
}

Then install statichceck HEAD:

$ go install honnef.co/go/tools/cmd/staticcheck@dec278f

Then see two different kinds of failures. The first is with an empty GOEXPERIMENT.

$ GOEXPERIMENT= staticcheck                                                                                                                                                                 ~/scratch/maps-all
../../sdk/go1.23rc1/src/maps/iter.go:51:20: cannot range over seq (variable of type iter.Seq2[K, V]) (compile)
../../sdk/go1.23rc1/src/slices/iter.go:51:17: cannot range over seq (variable of type iter.Seq[E]) (compile)

Notice with the above: it is complaining about iter.go in the standard library, not the use of rangefunc in main.go.

$ GOEXPERIMENT=rangefunc staticcheck                                                                                                                                                        ~/scratch/maps-all
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x28 pc=0x70fb72]

goroutine 231 [running]:
honnef.co/go/tools/go/ir.memberFromObject(0xc00028e630, {0x0, 0x0}, {0xa484b0, 0xc0002514d0}, {0x0, 0x0})
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/go/ir/create.go:53 +0x52
honnef.co/go/tools/go/ir.membersFromDecl(0xc00028e630, {0xa49ba0?, 0xc0002514d0}, {0x0, 0x0})
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/go/ir/create.go:165 +0x11c
honnef.co/go/tools/go/ir.(*Program).CreatePackage(0xc0004204e0, 0xc000282ae0, {0xc000121300, 0x1, 0x1}, 0xc000282b40, 0x0)
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/go/ir/create.go:212 +0x845
honnef.co/go/tools/internal/passes/buildir.run(0xc0001fa540)
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/internal/passes/buildir/buildir.go:85 +0x16e
honnef.co/go/tools/lintcmd/runner.(*analyzerRunner).do(0xc0005a78c0, {0xa4c178?, 0xc00039f2c0})
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/lintcmd/runner/runner.go:986 +0x71b
honnef.co/go/tools/lintcmd/runner.genericHandle({0xa4c178, 0xc00039f2c0}, {0xa4c178?, 0xc00039f180?}, 0xc000282ea0, 0xc0003f23b0, 0xc000528270)
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/lintcmd/runner/runner.go:811 +0x11f
created by honnef.co/go/tools/lintcmd/runner.(*subrunner).runAnalyzers in goroutine 97
	/home/matt/go/pkg/mod/honnef.co/go/[email protected]/lintcmd/runner/runner.go:1055 +0x6a6

Additional info

$ statichceck --version
staticcheck (devel, v0.5.0-0.dev.0.20240624124555-dec278f2f0d9)
$ staticcheck (devel, v0.5.0-0.dev.0.20240624124555-dec278f2f0d9)

Compiled with Go version: go1.22.3 X:rangefunc
Main module:
	honnef.co/go/[email protected] (sum: h1:QlANIrV9ZGfD4TrObBkwYAoRVO4u/PvISpkw8gCl1/Y=)
Dependencies:
	github.com/BurntSushi/[email protected] (sum: h1:pxW6RcqyfI9/kWtOwnv/G+AzdKuy2ZrqINhenH4HyNs=)
	golang.org/x/exp/[email protected] (sum: h1:1P7xPZEwZMoBoz0Yze5Nx2/4pxj6nw9ZqHWXqP0iRgQ=)
	golang.org/x/[email protected] (sum: h1:zY54UmvipHiNd+pm+m0x9KhZ9hl1/7QNMyxXbc6ICqA=)
	golang.org/x/[email protected] (sum: h1:YsImfSBoP9QPYL0xyKJPq0gcaJdG3rInoqxTWbfQu9M=)
	golang.org/x/[email protected] (sum: h1:SHq4Rl+B7WvyM4XODon1LXtP7gcG49+7Jubt1gWWswY=)
$ go version
go version go1.23rc1 linux/amd64
@matta matta added bug needs-triage Newly filed issue that needs triage labels Jun 29, 2024
@matta
Copy link
Author

matta commented Jun 29, 2024

@dominikh
Copy link
Owner

Note that when you do go install honnef.co/go/tools/cmd/staticcheck@dec278f, it is not upgrading Go to the version in your module. You're actually compiling Staticcheck with go1.22.3 X:rangefunc.

I don't yet know why this panics, and it shouldn't, but it works fine when you build Staticcheck with Go 1.23.

@dominikh
Copy link
Owner

The crash was the symptom of a missing error check, fixed in 5950450. f4ee291 improves Staticcheck to print a more useful error message when the version of Go Staticcheck was built with is older than the version required by the module under analysis.

That go install is not using your module's Go version is documented in go help install:

If the arguments have version suffixes (like @latest or @v1.0.0), "go install"
builds packages in module-aware mode, ignoring the go.mod file in the current
directory or any parent directory, if there is one. This is useful for
installing executables without affecting the dependencies of the main module.

That this makes the combination of go version + go install confusing is being discussed at golang/go#66518.

@dominikh dominikh removed the needs-triage Newly filed issue that needs triage label Jun 29, 2024
@matta
Copy link
Author

matta commented Jun 30, 2024

Note that when you do go install honnef.co/go/tools/cmd/staticcheck@dec278f, it is not upgrading Go to the version in your module. You're actually compiling Staticcheck with go1.22.3 X:rangefunc.

Ahh, I did not realize that these tools needed to be built with a version of go at least as recent as the go module they are analyzing. Makes sense, given that they are probably using huge portions of the compiler infrastructure.

f4ee291 improves Staticcheck to print a more useful error message when the version of Go Staticcheck was built with is older than the version required by the module under analysis.

Nice idea.

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

No branches or pull requests

2 participants