Skip to content

x/tools/internal/refactor/inline: avoid unnecessary interface conversions #68554

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

Closed
adonovan opened this issue Jul 23, 2024 · 1 comment
Closed
Assignees
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. Refactoring Issues related to refactoring tools Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Jul 23, 2024

This contrived example demonstrates that the source-level inliner (activated through gopls' refactor.inline code action) adds conversions to interface types even when they are not necessary:

func f(d discard) {
	g(d) // inlining this => fmt.Println(io.Writer(d)), where fmt.Println(d) would do
}

func g(w io.Writer) { fmt.Println(w) }

var d discard
type discard struct{}
func (discard) Write(p []byte) (int, error) { return len(p), nil }

Because fmt.Println's arguments are each converted to any, the implicit conversion from discard to io.Writer is not significant within the body of g and could be omitted. By contrast, were fmt.Println replaced by panic, the conversion would not be safe to remove, because in general panic(x) is not equivalent to panic(any(x)).

The inliner should detect, for each interface-typed parameter of the callee, whether all its uses in the callee's body are converted explicitly or implicitly to an interface type, so that the caller-side interface conversion may be safely omitted.

Googlers: see internal issue b/354664998

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jul 23, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jul 23, 2024
@dmitshur dmitshur added the NeedsFix The path to resolution is known, but the work has not been done. label Jul 25, 2024
@findleyr findleyr modified the milestones: Unreleased, gopls/backlog Nov 14, 2024
@findleyr findleyr added the Refactoring Issues related to refactoring tools label Nov 14, 2024
@findleyr findleyr self-assigned this Nov 19, 2024
@findleyr findleyr modified the milestones: gopls/backlog, gopls/v0.17.0 Nov 19, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/629875 mentions this issue: internal/refactor/inline: avoid unnecessary interface conversions

@findleyr findleyr added the FixPending Issues that have a fix which has not yet been reviewed or submitted. label Nov 20, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. 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

4 participants