golang / go

The Go programming language
https://go.dev
BSD 3-Clause "New" or "Revised" License
124.24k stars 17.7k forks source link

go/types,cmd/compile/internal/types2: extend types.Info with implicit conversions from assignability #47151

Open mdempsky opened 3 years ago

mdempsky commented 3 years ago

Currently, a statement like a = b may require an implicit conversion of b to a's type before the assignment can happen. In particular, for conversions to interface type, these generally require a run-time operation to change the value representation. (An implicit conversion can also happen for assignments between defined types and their underlying type literal or assignments from unrestricted channel types to send/receive-only channel types; but these are more of a formality, since Go implementations in practice use identical value representations for these cases.)

go/types provides enough information for users to infer/reconstruct these implicit conversions, but go/types already has to handle this as part of type checking anyway. So we might as well surface the information to users, so they don't have to remember all of the cases where implicit conversions happen.

My thoughts were along the lines of either:

  1. Add a types.Info.ImplicitTypes map of type map[ast.Expr]types.Type, which will record the type that the expression needs to be implicitly converted, if it differs from types.Info.Types[e].Type. (E.g., for var x int = int(42), int(42) would not need an entry in this map, because int(42) is already the same type as it's assigned to; but var x interface{} = int(42) would have an entry.)
  2. Same idea, but extend types.TypeAndValue with an extra Implicit Type field. Might be simpler/cleaner in some use cases, but it also implies more memory consumption (which I think at least gopls is very sensitive to).

Further, for N:1 assignments, I'd suggest the ImplicitTypes be present and a synthesized Tuple type if any of the respective tuple elements need implicit conversion. For example:

type MyBool bool
var f func() (int, int)
var g func(interface{}, interface{})
var a int
var b interface{}
var x MyBool

a, b = f() // 'f()' has Type (int, int); but ImplicitType (int, interface{}), due to assignment to a,b

g(f()) // 'f()' has Type (int, int); but ImplicitType (interface{}, interface{}), due to passing to g()

b, x = b.(int) // 'b.(int)' has Type (int, MyBool†); but ImplicitType (interface{}, MyBool), due to assignment to b,x
// † cmd/compile's legacy typechecker produces 'MyBool' in this case, but I think 'bool' or 'untyped bool' would be okay too

Incidentally, I think having a way to record separate types for the expression in isolation vs expression in context could help with cases like #13061.

/cc @griesemer @findleyr

mdempsky commented 3 years ago

/cc @dominikh too

griesemer commented 3 years ago

Seems ok at first glance. @findleyr for issues with API backward-compatibility.

findleyr commented 3 years ago

I think this makes sense. A big chunk of gopls' memory footprint is types.TypeAndValue, so yes I'd argue against adding a word there, but adding an additional map to types.Info should be fine. I don't see any compatibility issues with the additional map, though fully addressing #13061 can introduce some tricky compatibilty problems (example).

Bikeshedding: I used 'implicit type' in CL 242084, but I'm not sure if this phrasing was used before then. Just pointing out that there may not be consensus on that naming, and we might want to re-evaluate before committing it to an API. It might be confusing with the existing types.Info.Implicits. I'd still vote for it, though.

mdempsky commented 3 years ago

I don't see any compatibility issues with the additional map, though fully addressing #13061 can introduce some tricky compatibilty problems (example).

Fair warning. I think it would be nice if we can fix the inconsistencies around declared constants and nil, but I don't see it as essential either.

Bikeshedding: I used 'implicit type' in CL 242084, but I'm not sure if this phrasing was used before then. Just pointing out that there may not be consensus on that naming, and we might want to re-evaluate before committing it to an API. It might be confusing with the existing types.Info.Implicits. I'd still vote for it, though.

Ack, I don't love the name and am open to suggestions. What about ImplicitConversions or AssignedTypes? (as in, the type of the expression after implicit conversion due to assignment).

cuonglm commented 3 years ago

@mdempsky how about ImpliedTypes?

mdempsky commented 3 years ago

@mdempsky how about ImpliedTypes?

I was going to say "implied" doesn't appear in the spec, but actually it does: "A conversion may appear literally in the source, or it may be implied by the context in which an expression appears."

I think that works.

adonovan commented 1 year ago

This is essentially a dup of #8970. In other words, this feature would obviate the golang.org/x/tools/refactor/satisfy package.