go/types: missing Info.Types for *ast.FuncType

[adonovan]

In this program, types.Info.Types records no type for f's ast.FuncType node. I think it should:

package p

func f() (int, string) {
    return 0, "" 
}

@madelinekalil

[gabyhelp]

Related Issues

• go/types: missing Info.Types entry for variadic parameter *ast.Ellipsis #28277 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[Agent-Hellboy]

Hi @adonovan,

I’d like to work on this if @madelinekalil approve the issue. I’ve recently added a PR on a mini parser for go.mod, and the Go codebase looks clean and approachable. I am particularly interested in the parser, AST, and type-checking stages, and I want to contribute to these areas as they seem beginner-friendly and a great way to start learning a language's type system through hands-on experience.

patch:

diff --git a/src/go/types/signature.go b/src/go/types/signature.go
index 681eb85fd7..590e0a982b 100644
--- a/src/go/types/signature.go
+++ b/src/go/types/signature.go
@@ -106,12 +106,18 @@ func (s *Signature) String() string   { return TypeString(s, nil) }
 
 // funcType type-checks a function or method type.
 func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast.FuncType) {
        check.openScope(ftyp, "function")
        check.scope.isFunc = true
        check.recordScope(ftyp, check.scope)
        sig.scope = check.scope
        defer check.closeScope()
 
+       if check.Info.Types != nil {
+               check.recordTypeAndValue(ftyp, typexpr, sig, nil)
+       }
+
        // collect method receiver, if any
        var recv *Var
        var rparams *TypeParamList

[adonovan]

I’d like to work on this if @madelinekalil approve the issue. I’ve recently added a PR on a mini parser for go.mod, and the Go codebase looks clean and approachable. I am particularly interested in the parser, AST, and type-checking stages

Thanks for your interest! @madelinekalil discovered the bug, but I think @griesemer is the best person to decide whether the simple fix you suggest is the right one. I know in the past there have been several discussions about when exactly the type checker guarantees to record a type for an expression, and it's possible this one lies in a grey area since, in context, the FuncType it isn't strictly a term or a type. Perhaps the most significant part of the fix will be to clarify the documentation on precisely what the user can safely assume.

they seem beginner-friendly

I have never heard go/types described that way before! ;-)

[Agent-Hellboy]

but I think @griesemer is the best person to decide whether the simple fix you suggest is the right one.

no problem. I can understand, I will keep looking at the issues and PRs and learning.

I have never heard go/types described that way before! ;-)

No no, I mean I am interested, and at least I can understand issues associated with these layers.

[griesemer]

I looked into this in fair detail:

For this program:

package p

var x int
var y = 1.0

func f() (int, string) {
    return 0, ""
}

var g func()

we get the following type recordings (playground):

Types
	 p:7:12 	 0 	 {4 int 0}
	 p:7:15 	 "" 	 {4 string ""}
	 p:3:7 	 int 	 {3 int <nil>}
	 p:4:9 	 1.0 	 {4 float64 1}
	 p:6:11 	 int 	 {3 int <nil>}
	 p:6:16 	 string 	 {3 string <nil>}
	 p:10:7 	 func() 	 {3 func() <nil>}
Defs
	 p:10:5 	 g 	 var p.g func()
	 p:1:9 	 p 	 <nil>
	 p:3:5 	 x 	 var p.x int
	 p:4:5 	 y 	 var p.y float64
	 p:6:6 	 f 	 func p.f() (int, string)

The type for f is recorded (via an Object) in the Info.Defs map since f is a declaration, so the type information is definitely present. For the variable x we record the type in Info.Defs (as expected), but we also have the type int recorded in the Info.Types map since we have a (syntactic) type expression int.

Presumably, the argument is that since we record the type for the type expression int we should do the same for the signature "expressions" in function declarations.

Yet, we don't do it for function declarations, because there's no obvious syntactic function type expression present in the source (e.g. we don't write f func() (int string)), we mix the function type into the declaration. The ast.FuncDecl in th AST is artificial in that sense.

Note that we definitely record the function type in info.Types when we have an explicit syntactic function type (line 10).

I am not convinced we should make the suggested change. For one it has been working like this for more than a decade and nobody complained. Second, I am concerned that recording this information may cause unexpected bugs (due to tools not expecting the data). Finally, for generic functions, go/ast records type parameters with the ast.FuncType, but the syntax package records type parameters with the FuncDecl node, so there's no obvious 1:1 correspondence between a FuncType node and the respective types.Signature in that sense, exactly because syntactically we actually don't have a clear-cut function type for function declarations.

As an aside, the suggested fix doesn't look right; that type information is recorded externally, and doing it in Checker.funcType may lead to repeated recordings.

All in all, I believe we should leave things as they are.
@adonovan, @findleyr Thoughts?

[adonovan]

Yeah, I agree it's a grey area. I think a reasonable resolution would be to document more explicitly at Info.Types what the client can expect.

[gopherbot]

Change https://go.dev/cl/640776 mentions this issue: go/types, types2: expand documentation for Info.Types map

[findleyr]

It is probably simplest not to change this behavior, since this has been the case for a long time. Improving documentation sounds like a reasonable solution.

[gopherbot]

Change https://go.dev/cl/640596 mentions this issue: go/types, types2: don't register interface methods in Info.Types map