golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.29k stars 17.7k forks source link

x/tools/gopls: refactor.rewrite.removeUnusedParam: avoid literalization for recursive spread call #70554

Open tttoad opened 1 day ago

tttoad commented 1 day ago

gopls version

Build info

golang.org/x/tools/gopls v0.16.2 golang.org/x/tools/gopls@v0.16.2 h1:K1z03MlikHfaMTtG01cUeL5FAOTJnITuNe0TWOcg8tM= github.com/BurntSushi/toml@v1.2.1 h1:9F2/+DoOYIOksmaJFPw1tGFy1eDnIJXg+UHjuD8lTak= github.com/google/go-cmp@v0.6.0 h1:ofyhxvXcZhMsU5ulbFiLKl/XBFqE1GSq7atu8tAmTRI= golang.org/x/exp/typeparams@v0.0.0-20221212164502-fae10dda9338 h1:2O2DON6y3XMJiQRAS1UWU+54aec2uopH3x7MAiqGW 6Y= golang.org/x/mod@v0.20.0 h1:utOm6MM3R3dnawAiJgn0y+xvuYRsm1RKM/4giyfDgV0= golang.org/x/sync@v0.8.0 h1:3NFvSEYkUoMifnESzZl15y791HH1qU2xm6eCJU5ZPXQ= golang.org/x/telemetry@v0.0.0-20240829154258-f29ab539cc98 h1:Wm3cG5X6sZ0RSVRc/H1/sciC4AT6HAKgLCSH2lbpR/c= golang.org/x/text@v0.16.0 h1:a94ExnEXNtEwYLGJSIUxnWoxoRz/ZcCsV63ROupILh4= golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653 h1:6bJEg2w2kUHWlfdJaESYsmNfI1LKAZQi6zCa7LUn7eI= golang.org/x/vuln@v1.0.4 h1:SP0mPeg2PmGCu03V+61EcQiOjmpri2XijexKdzv8Z1I= honnef.co/go/tools@v0.4.7 h1:9MDAWxMoSnB6QoSqiVr7P5mtkT9pOc1kSxchzPCnqJs= mvdan.cc/gofumpt@v0.6.0 h1:G3QvahNDmpD+Aek/bNOLrFR2XC6ZAdo62dZu65gmwGo= mvdan.cc/xurls/v2@v2.5.0 h1:lyBNOm8Wo71UknhUs4QTFUNNMyxy2JEIaKKo0RWOh+8= go: go1.23.1

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/work/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/toad/work'
GOPRIVATE=''
GOPROXY='https://goproxy.cn,direct'
GOROOT='/Users/toad/go/go1.23.1/'
GOSUMDB='sum.golang.org'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/Users/toad/go/go1.23.1/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.23.1'
GODEBUG=''
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/toad/Library/Application Support/go/telemetry'
GCCGO='gccgo'
GOARM64='v8.0'
AR='ar'
CC='cc'
CXX='clang++'
CGO_ENABLED='1'
GOMOD='/Users/toad/work/demo10/go.mod'
GOWORK=''
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
PKG_CONFIG='pkg-config'
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-pref
ix-map=/var/folders/g1/tgmnlrdn3vxgv08kdgh9vpkw0000gn/T/go-build4008461701=/tmp/go-build -gno-record-gcc-switch
es -fno-common'

What did you do?

package main

func CC(aa, bb int) { // Remove useless parameters bb.
    CC(BB(aa))
}

func BB(aa int) (int, int) {
    return 0, 0
}

What did you expect to see?

report an error.

What did you see happen?

func CC(aa int) {
     func(aa, _ int) {
        CC(aa)
    }(BB(aa))
}

func BB(aa int) (int, int) {
    return 0, 0
}

I see "A better solution would be to allow for ad-hoc snapshots that expose the full machinery of real snapshots: minimal invalidation, batched type checking, etc. " in the comments of the inline_all.inlineAllCalls. Maybe we should implement it. "Extract parameter struct" will also use it.

Editor and settings

No response

Logs

No response

gabyhelp commented 1 day ago

Related Issues

(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)

findleyr commented 15 hours ago

The resulting code looks correct. We could do a better job of eliminating the literalization, but the code is not wrong.