Skip to content

x/tools/internal/refactor/inline: panic in substitute when removing unused parameter #70268

Closed
@tttoad

Description

@tttoad

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-build2268726973=/tmp/go-build -gno-record-gcc-switch
es -fno-common'

What did you do?

ddd/ddd.go

package ddd

func Search(aa, bb int) {  // refactor: remove unused parameter. 
}

main.go

package main

import (
	"demo/ddd"
)

func main() {
	ddd.Search(1)  // 1. not enough arguments in call to ddd.Search have (number) want (int, int) 
}

What did you see happen?

panic: runtime error: index out of range [1] with length 1
"
goroutine 1205 gp=0x14000bb2000 m=2 mp=0x14000076808 [running]:
panic({0x1019d6220?, 0x14000447da0?})
\t/Users/toad/go/go1.23.1/src/runtime/panic.go:804 +0x154 fp=0x140006fb530 sp=0x140006fb480 pc=0x100c52ab4
runtime.goPanicIndex(0x1, 0x1)
\t/Users/toad/go/go1.23.1/src/runtime/panic.go:115 +"0x74 fp=0x140006fb570 sp=0x140006fb530 pc=0x100c16894
golang.org/x/tools/internal/refactor/inline.substitute(0x101a34930, 0x140004d7380, {0x14001011790, 0x2, 0x2}, {0x1400040aa40, 0x1, 0x1}, {0x1400000f6c0, 0x2, ...}, ...)
"\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/refactor/inline/inline.go:1350 +0xea0 fp=0x140006fb770 sp=0x140006fb570 pc=0x101438210
golang.org/x/tools/internal/refactor/inline.(*state).inlineCall(0x140006fc210)
\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/refactor/inline/inline.go:737 +0x14b8 fp=0x140006fbf90 sp=0x140006fb770 pc=0x101433408
golang.org/x/tools/internal/refactor/inline.(*state).inline(0x140006fc210)
\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/refactor/inline/inline.go:105 +0x1d8 fp=0x140006fc1f0 sp=0x140006fbf90 pc=0x101430e28
golang.org/x/tools/internal/refactor/inline.Inline(0x140004d7380, 0x140001ba140, 0x14001011600)
\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/refactor/inline/inline.go:78 +0x9c fp=0x140006fc230 sp=0x140006fc1f0 pc=0x101430c1c
"golang.org/x/tools/gopls/internal/golang.inlineAllCalls({0x101a43180, 0x140007d4c80}, 0x101a34930, 0x14000c1e360, 0x14000b7cb10, 0x1400061ea10, 0x140007c2a50, 0x140001ba140, 0x101a34928)
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/golang/inline_all.go:196 +0xf78 fp=0x140006fcab0 sp=0x140006fc230 pc=0x10147e6f8
golang.org/x/tools/gopls/internal/golang.rewriteCalls({0x101a43180, 0x140007d4c80}, {0x14000c1e360, 0x14000b7cb10, 0x1400061ea10, 0x140007c2a50, 0x140007c3560, 0x140007c3680, {0x14001010d50, 0x1, ...}, ...})
"\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/golang/change_signature.go:471 +0x904 fp=0x140006fce00 sp=0x140006fcab0 pc=0x101450b04
golang.org/x/tools/gopls/internal/golang.RemoveUnusedParameter({0x101a43180, 0x140007d4c80}, {0x101a45ec0?, 0x140007d0000?}, {{0x492f2e48?, 0x1?}, {0x7d4cd0?, 0x140?}}, 0x14000c1e360)
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/golang/change_signature.go:124" +0x668 fp=0x140006fd230 sp=0x140006fce00 pc=0x10144f018
golang.org/x/tools/gopls/internal/server.(*commandHandler).ChangeSignature.func1({0x101a43180, 0x140007d4c80}, {0x14000c1e360?, {0x101a45ec0?, 0x140007d0000?}, 0x0?"})
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/server/command.go:1338 +0x6c fp=0x140006fd2d0 sp=0x140006fd230 pc=0x1014f151c
golang.org/x/tools/gopls/internal/server.(*commandHandler).run.func2()
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/server/command.go":174 +0x80 fp=0x140006fd380 sp=0x140006fd2d0 pc=0x1014e7720
golang.org/x/tools/gopls/internal/server.(*commandHandler).run(0x14001010c10, {0x101a43148, 0x140007c3230}, {0x0", {0x0, 0x0}, {0x0, 0x0}, {"0x1400030a780, 0x28}}, ...)
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/server/command.go:201 +0x494 fp=0x140006fd4b0 sp=0x140006fd380 pc=0x1014e73c4"
golang.org/x/tools/gopls/internal/server.(*commandHandler).ChangeSignature(0x14001010c10, {0x101a43148, 0x140007c3230}, {{{0x1400030a780, 0x28}, "{{0x2, 0xd}, {0x2, 0xd}}"}, 0x1})
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/server/command.go:1334 +0x130 fp=0x140006fd560" sp=0x140006fd4b0 pc=0x1014f1430
golang.org/x/tools/gopls/internal/protocol/command.Dispatch({0x101a43148, 0x140007c3230}, 0x14000b4bbc0, {"0x101a57e98, 0x14001010c10})
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/protocol/command/command_gen.go:145 +0x1808 fp=0x140006fd850 sp=0x140006fd560 pc=0x10128f0c8
golang.org/x/tools/gopls/internal/server.(*server).ResolveCodeAction(0x14000208780, {0x101a43180?", 0x1400009b040?}, 0x1400051f500)
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/server/code_action.go:190 +0x180 fp=0x140006fd8f0 sp=0x140006fd850" pc=0x1014e4e20
golang.org/x/tools/gopls/internal/protocol.serverDispatch({0x101a43180, 0x1400009b040}, {0x101a5e1c0, "0x14000208780}, 0x140007c3170, {0x101a43340, 0x140004e8f80})
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/protocol/tsserver.go":214 +0xb14 fp=0x140006fdbd0 sp=0x140006fd8f0 pc=0x100f7ecf4
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.ServerHandler.func3"({0x101a43180, 0x1400009b040}, 0x140007c3170, {0x101a43340, 0x140004e8f80})
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/protocol/protocol.go:"160 +0x74 fp=0x140006fdc30 sp=0x140006fdbd0 pc=0x101510ca4
golang.org/x/tools/gopls/internal/lsprpc.(*streamServer).ServeStream.handshaker.func4({"0x101a43180, 0x1400009b040}, 0x140007c3170, {0x101a43340, "0x140004e8f80})
\t/Users/toad/work/pkg/mod/golang.org/x/tools/gopls@v0.16.2/internal/lsprpc/lsprpc.go:509 +0x6b8" fp=0x140006fdee0 sp=0x140006fdc30 pc=0x101510ab8
golang.org/x/tools/gopls/internal/protocol.Handlers.MustReplyHandler.func1({"0x101a43180, 0x1400009b040}, 0x1400042ac00, {0x101a43340, 0x140004e8f80})
\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/jsonrpc2/handler.go:35 +0xc0 fp=0x140006fdf40 sp=0x140006fdee0" pc=0x100f6ffb0
golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2.2()
\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/jsonrpc2/handler.go:103 +0x90" fp=0x140006fdfd0 sp=0x140006fdf40 pc=0x100f6fde0
runtime.goexit({})
\t/Users/toad/go/go1.23.1/src/runtime/asm_arm64.s:"1223 +0x4 fp=0x140006fdfd0 sp=0x140006fdfd0 pc=0x100c5bc84
created by golang.org/x/tools/gopls/internal/protocol.Handlers.AsyncHandler.func2" in goroutine 80
\t/Users/toad/work/pkg/mod/golang.org/x/tools@v0.22.1-0.20240829175637-39126e24d653/internal/jsonrpc2/handler.go:100 +0x19c

What did you expect to see?

Report an error, not panic.

func substitute(logf func(string, ...any), caller *Caller, params []*parameter, args []*argument, effects []int, falcon falconResult, replaceCalleeID func(offset int, repl ast.Expr)) {
	// Inv:
	//  in        calls to     variadic, len(args) >= len(params)-1
	//  in spread calls to non-variadic, len(args) <  len(params)
	//  in spread calls to     variadic, len(args) <= len(params)
	// (In spread calls len(args) = 1, or 2 if call has receiver.)
	// Non-spread variadics have been simplified away already,
	// so the args[i] lookup is safe if we stop after the spread arg.
next:
	for i, param := range params {
		arg := args[i]  // panic
        // ...
       }
}

Editor and settings

No response

Logs

No response

Activity

added
goplsIssues related to the Go language server, gopls.
ToolsThis label describes issues relating to any tools in the x/tools repository.
on Nov 9, 2024
added this to the Unreleased milestone on Nov 9, 2024
gopherbot

gopherbot commented on Nov 9, 2024

@gopherbot
Contributor

Change https://go.dev/cl/626895 mentions this issue: gopls/codeaction: fix panic when removing unused parameters with syntax errors.

changed the title [-]x/tools/gopls: panic when removing parameters.[/-] [+]x/tools/internal/refactor/inline: panic in substitute when removing unused parameter[/+] on Nov 10, 2024
tttoad

tttoad commented on Nov 11, 2024

@tttoad
Author

@adonovan I think we can avoid panic by diagnosing to ignore calls with syntax errors.

findleyr

findleyr commented on Nov 11, 2024

@findleyr
Member

Thanks @tttoad, I'd be fine with that, though see my comments on your CL about how to implement this.

added a commit that references this issue on Nov 14, 2024
5b5d57c

1 remaining item

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Labels

RefactoringIssues related to refactoring toolsToolsThis 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

      @adonovan@gopherbot@tttoad@findleyr@gabyhelp

      Issue actions

        x/tools/internal/refactor/inline: panic in substitute when removing unused parameter · Issue #70268 · golang/go