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: add analyzer to simplify slice to array conversions #69820

Open
dsnet opened this issue Oct 9, 2024 · 6 comments
Open

x/tools/gopls: add analyzer to simplify slice to array conversions #69820

dsnet opened this issue Oct 9, 2024 · 6 comments
Labels
gopls Issues related to the Go language server, gopls. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@dsnet
Copy link
Member

dsnet commented Oct 9, 2024

Go version

go1.23.1

Output of go env in your module/workspace:

n/a

What did you do?

Run gofmt -s

What did you see happen?

Nothing.

What did you expect to see?

Rewrite of *(*logid.PublicID)(buf) to logid.PublicID(buf). Where logid.PublicID is an [...]byte type and buf is an []byte.

Go 1.17 introduced conversion of slices to array pointers, which resulted in code of the *(*A)(s) pattern. Go 1.20 introduced direct conversion of a slice to an array, thus making the *(*A)(s) pattern redundant.

@mateusz834
Copy link
Member

Rewrite of *(*logid.PublicID)(buf) to logid.PublicID(buf). Where logid.PublicID is an [...]byte type and buf is an []byte.

I might be wrong, but this change requires type information, but the gofmt currently is only executed on the AST, so it might not be possible to do so, right?

@dsnet
Copy link
Member Author

dsnet commented Oct 9, 2024

Ah, yes. I believe you are correct. This transformation does indeed require type information.

@cherrymui cherrymui changed the title gofmt: simplify slice to array conversions cmd/gofmt: simplify slice to array conversions Oct 9, 2024
@cherrymui
Copy link
Member

From the discussion this sounds infeasible (with the current design)?

cc @griesemer @mvdan to decide.

@cherrymui cherrymui added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Oct 9, 2024
@cherrymui cherrymui added this to the Backlog milestone Oct 9, 2024
@zigo101
Copy link

zigo101 commented Oct 9, 2024

*(*logid.PublicID)(buf) is not always equivalent to logid.PublicID(buf), at least when it is used as a L-value.

@zigo101
Copy link

zigo101 commented Oct 9, 2024

I'm not sure whether or not the (*(*[2]int)(x)) expression in the following code should be a L-values.
But the code compiles, which indicates they are L-values.

package main

import (
	"fmt"
)

func main() {
	{
		var x = []int{1, 2}
		for i, v := range (*(*[2]int)(x))[:] {
			x[1] = 9
			if (i == 1) {
				fmt.Println(v) // 9
			}
		}
	}
	{
		var x = []int{1, 2}
		var p = &(*(*[2]int)(x))[1]
		*p = 9
		fmt.Println(x[1]) // 9
	}
}

@adonovan
Copy link
Member

While not appropriate for vet, this could be added to one of the "simplify" analyzers within gopls. I'll change the title.

To recap, the pattern is:

var slice []T
const k
*(*[k]T)(slice)  =>  [k]T(slice)  // when used as rvalue

@adonovan adonovan changed the title cmd/gofmt: simplify slice to array conversions x/tools/gopls: add analyzer to simplify slice to array conversions Oct 12, 2024
@gopherbot gopherbot added Tools This label describes issues relating to any tools in the x/tools repository. gopls Issues related to the Go language server, gopls. labels Oct 12, 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. NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants