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

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

Open
tttoad opened this issue Nov 25, 2024 · 2 comments
Assignees
Labels
gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@tttoad
Copy link

tttoad commented Nov 25, 2024

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

@tttoad tttoad added gopls Issues related to the Go language server, gopls. Tools This label describes issues relating to any tools in the x/tools repository. labels Nov 25, 2024
@gopherbot gopherbot added this to the Unreleased milestone Nov 25, 2024
@adonovan adonovan changed the title x/tools/gopls: Error when removing a useless parameter, the self-call parameters are the values of another function. x/tools/gopls: refactor.rewrite.removeUnusedParam: self-call parameters are the values of another function Nov 25, 2024
@adonovan adonovan changed the title x/tools/gopls: refactor.rewrite.removeUnusedParam: self-call parameters are the values of another function x/tools/gopls: refactor.rewrite.removeUnusedParam: invalid result in presence of recursive spread call Nov 25, 2024
@adonovan adonovan added the Refactoring Issues related to refactoring tools label Nov 25, 2024
@findleyr findleyr modified the milestones: Unreleased, gopls/v0.17.0 Nov 25, 2024
@findleyr findleyr self-assigned this Nov 25, 2024
@findleyr
Copy link
Member

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

@findleyr findleyr changed the title x/tools/gopls: refactor.rewrite.removeUnusedParam: invalid result in presence of recursive spread call x/tools/gopls: refactor.rewrite.removeUnusedParam: avoid literalization for recursive spread call Nov 26, 2024
@findleyr findleyr modified the milestones: gopls/v0.17.0, gopls/v0.18.0 Nov 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gopls Issues related to the Go language server, gopls. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

5 participants