x/tools/internal/refactor/inline: compute minimum version from callee code

[adonovan]

When inlining a call from one file to another, it's possible that the callee uses a newer feature of the Go spec or library than the caller, causing inlining to break the caller. The tool should compute the minimum language version implied by the callee function body (e.g. by checking each std symbol it references, or whether it uses a feature such as new(expr)) and refuse to inline if the caller is older than that.

This could be quite tricky in the general case.

[findleyr]

This is a nice idea. Pragmatically, we may need type checker support.

CC @griesemer @mrkfrmn -- what do you think about recording "minimum required version" information during type checking? It would be relatively easy to implement -- just hook into allowVersion -- though I have no idea what the types.Info map key would be for this, since we want to be able to compute this at the level of function bodies for the inliner. Maybe map[Expr]XXX?

[griesemer]

Interesting idea. Tracking the minimum version via allowVersion for one function shouldn't be too hard. But presumably one would also have to determine the entire call tree for each exported function and it's minimum version, since the inliner may go deeper than just one call? The devil is in the details, as usual.

[adonovan]

I have no idea what the types.Info map key would be for this, since we want to be able to compute this at the level of function bodies for the inliner. Maybe map[Expr]XXX?

It's not just expressions, it's every Node (e.g. does a FuncDecl have type params?). I would rather not add the expense of another map holding every Node just for this esoteric feature. It would be better to record just the few nodes that establish a version dependency, and to make the client compute the sum of constraints across an entire tree. That argues for a sparse map (or encapsulation of the same).

[mrkfrmn]

Agree that it sounds like a nice idea — I imagine if every function has only a shallow notion of its minimum version (not the functions it calls), then the inliner could just take that into account when it determines how aggressively to inline.

I assume we can say any syntactic appearance of a version establishing Node propagates the version establishment — having to examine actual usage seems harder.

[adonovan]

To be clear, the inliner in question here is the source-level inliner (refactoring tool), not the compiler's inliner (optimization).

[findleyr]

That argues for a sparse map (or encapsulation of the same).

To be clear, I was imagining a sparse map, leaving the computation to the user.

[findleyr]

Ok, so something like MinVersions map[ast.Node]string mapping sparse nodes to the minimum Go version they require.

@adonovan we're about to add go:fix inline for 1.26. Should we prioritize a proposal to support this issue?
It's important that go fix is super accurate, and I can imagine this being a nontrivial source of inaccuracy.

[findleyr]

@griesemer and @mrkfrmn, as Alan said: since this is just about source inlining, it really is enough to record sparse, shallow information associated with the AST.

[griesemer]

Understood. It seems that function-level information is all we need, unless I am missing something. Maybe, for flexibility, this could be information attached (via an extra map) to any top-level object that can be exported, even if for a start we only have functions.

[findleyr]

this could be information attached (via an extra map) to any top-level object that can be exported.

FWIW, unexported functions matter too: the build could have two different files in the same package with different minimum Go versions.

I think there's a non-trivial design decision for the map: if we make the key e.g. ast.Decl, that may reduce its utility, though I can't think of an example.

@adonovan suggests that we try to implement to get a better understanding of the DX. He or I can do this.

[gopherbot]

Change https://go.dev/cl/704820 mentions this issue: gopls/internal/analysis/modernize: pass to use go1.26 new(x)