Skip to content

x/tools/gopls: No warning "loop variable xx captured by func literal". #60016

Not planned
@tttoad

Description

@tttoad

gopls version

golang.org/x/tools/gopls v0.11.0
golang.org/x/tools/gopls@v0.11.0 h1:/nvKHdTtePQmrv9XN3gIUN9MOdUrKzO/dcqgbG6x8EY=
github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak=
github.com/google/go-cmp@v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38=
github.com/sergi/go-diff@v1.1.0 h1:we8PVUC3FE2uYfodKH/nBHMSetSfHDR6scGdBi+erh0=
golang.org/x/exp@v0.0.0-20221031165847-c99f073a8326 h1:QfTh0HpN6hlw6D3vu8DAwC8pBIwikq0AI1evdm+FksE=
golang.org/x/exp/typeparams@v0.0.0-20221031165847-c99f073a8326 h1:fl8k2zg28yA23264d82M4dp+YlJ3ngDcpuB1bewkQi4=
golang.org/x/mod@v0.7.0 h1:LapD9S96VoQRhi/GrNTqeBJFrUjs5UHCAtTlgwA5oZA=
golang.org/x/sync@v0.1.0 h1:wsuoTGHzEhffawBOhz5CYhcrV4IdKZbEyZjBMuTp12o=
golang.org/x/sys@v0.2.0 h1:ljd4t30dBnAvMZaQCevtY0xLLD0A+bRZXbgLMLU1F/A=
golang.org/x/text@v0.4.0 h1:BrVqGRd7+k1DiOgtnFvAkoQEWQvBc25ouMJM6429SFg=
golang.org/x/tools@v0.3.1-0.20221213193459-ca17b2c27ca8 h1:7/HkGkN/2ktghBCSRRgp31wAww4syfsW52tj7yirjWk=
golang.org/x/vuln@v0.0.0-20221109205719-3af8368ee4fe h1:qptQiQwEpETwDiz85LKtChqif9xhVkAm8Nhxs0xnTww=
honnef.co/go/tools@v0.3.3 h1:oDx7VAwstgpYpb3wv0oxiZlxY+foCpRAwY7Vk6XpAgA=
mvdan.cc/gofumpt@v0.4.0 h1:JVf4NN1mIpHogBj7ABpgOyZc65/UUOkKQFkoURsz4MM=
mvdan.cc/xurls/v2@v2.4.0 h1:tzxjVAj+wSBmDcF6zBB7/myTy3gX9xvi8Tyr28AuQgc=
go: go1.19

go env

GO111MODULE=""
GOARCH="arm64"
GOBIN=""
GOCACHE="/Users/toad/Library/Caches/go-build"
GOENV="/Users/toad/Library/Application Support/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="arm64"
GOHOSTOS="darwin"
GOINSECURE=""
GOMODCACHE="/Users/toad/go/go1.19/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="darwin"
GOPATH="/Users/toad/go/go1.19"
GOPRIVATE=""
GOPROXY="https://goproxy.cn,direct"
GOROOT="/opt/homebrew/Cellar/go/1.19/libexec"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/opt/homebrew/Cellar/go/1.19/libexec/pkg/tool/darwin_arm64"
GOVCS=""
GOVERSION="go1.19"
GCCGO="gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/Users/toad/work/gopls-demo/go.mod"
GOWORK=""
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/var/f
olders/g1/tgmnlrdn3vxgv08kdgh9vpkw0000gn/T/go-build537849748=/tmp/go-build -gno-record-gcc-switches -fno-common"

What did you do?

package main

import (
	"fmt"
)

func main() {
	al := make([]string, 0)
	al = append(al, "123", "456")

	for _, a := range al {
		go func() {
			fmt.Println(a)
		}()

		go func(a string) {
			fmt.Println(a)
		}(a)
	}
}

What did you expect to see?

in line 13: warning, loop variable a captured by func literal.

What did you see instead?

2023-05-06.10.18.06.mov

Activity

added
ToolsThis label describes issues relating to any tools in the x/tools repository.
goplsIssues related to the Go language server, gopls.
on May 6, 2023
added this to the Unreleased milestone on May 6, 2023
findleyr

findleyr commented on May 8, 2023

@findleyr
Member

This is working as intended. The loopclosure analyzer only catches references in go statements that are the last statement in a loop body. Otherwise, the goroutine could be synchronized by a number of means, causing the analyzer to produce false positives.

Knowing whether or not the loop value escapes the loop iteration is a surprisingly tricky problem.

Note that there is discussion about changing loop variable semantics in #56010.

CC @adonovan @timothy-king

timothy-king

timothy-king commented on May 8, 2023

@timothy-king
Contributor

This specific example can be caught by an alternative definition of "last statements". Here is an example WiP that IIRC can detect this specific example: https://go-review.git.corp.google.com/c/tools/+/455195 . The 'lastness' is modified 'statement must be able to reach the end of the loop'. These definitions are unfortunately a bit hard to predict. It still does not take much to stop such analyzers. Any expression that may panic or function call stops going backwards.

func foo() {
	...
	for _, a := range al {
		go func() {
			fmt.Println(a)
		}()
		extraCall() // extra function call that can panic or potentially synchronize
		go func(a string) {
			fmt.Println(a)
		}(a)
	}
}

Neither syntactic definition ends up entirely satisfying.

Efforts in this direction may end up somewhat moot due to #56010.

locked and limited conversation to collaborators on May 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    FrozenDueToAgeToolsThis label describes issues relating to any tools in the x/tools repository.goplsIssues related to the Go language server, gopls.

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @timothy-king@gopherbot@tttoad@findleyr

        Issue actions

          x/tools/gopls: No warning "loop variable xx captured by func literal". · Issue #60016 · golang/go