[go] cmd/cgo: handle implicitly-typed compound struct literals

[David Chase (Gerrit)]

David Chase has uploaded the change for review

Commit message

cmd/cgo: handle implicitly-typed compound struct literals

There are almost certainly corner cases that this doesn't cover,
but fewer than before.

Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Change diff

diff --git a/src/cmd/cgo/ast.go b/src/cmd/cgo/ast.go
index dac7151f..4b3fc3b 100644
--- a/src/cmd/cgo/ast.go
+++ b/src/cmd/cgo/ast.go
@@ -162,8 +162,8 @@
 	if f.Ref == nil {
 		f.Ref = make([]*Ref, 0, 8)
 	}
-	f.walk(ast2, ctxProg, (*File).validateIdents)
-	f.walk(ast2, ctxProg, (*File).saveExprs)
+	f.walk(ast2, ctxProg, nilTC, (*File).validateIdents)
+	f.walk(ast2, ctxProg, nilTC, (*File).saveExprs)
 
 	// Accumulate exported functions.
 	// The comments are only on ast1 but we need to
@@ -171,8 +171,8 @@
 	// The first walk fills in ExpFunc, and the
 	// second walk changes the entries to
 	// refer to ast2 instead.
-	f.walk(ast1, ctxProg, (*File).saveExport)
-	f.walk(ast2, ctxProg, (*File).saveExport2)
+	f.walk(ast1, ctxProg, nilTC, (*File).saveExport)
+	f.walk(ast2, ctxProg, nilTC, (*File).saveExport2)
 
 	f.Comments = ast1.Comments
 	f.AST = ast2
@@ -199,7 +199,7 @@
 	return strings.Join(pieces, "")
 }
 
-func (f *File) validateIdents(x interface{}, context astContext) {
+func (f *File) validateIdents(x interface{}, context astContext, typeOf typeContext) {
 	if x, ok := x.(*ast.Ident); ok {
 		if f.isMangledName(x.Name) {
 			error_(x.Pos(), "identifier %q may conflict with identifiers generated by cgo", x.Name)
@@ -208,7 +208,7 @@
 }
 
 // Save various references we are going to need later.
-func (f *File) saveExprs(x interface{}, context astContext) {
+func (f *File) saveExprs(x interface{}, context astContext, typeOf typeContext) {
 	switch x := x.(type) {
 	case *ast.Expr:
 		switch (*x).(type) {
@@ -218,7 +218,7 @@
 	case *ast.CallExpr:
 		f.saveCall(x, context)
 	case *ast.CompositeLit:
-		f.saveLiteral(x, context)
+		f.saveLiteral(x, context, typeOf)
 	}
 }
 
@@ -266,7 +266,7 @@
 	})
 }
 
-// Save calls to C.xxx for later processing.
+// saveCalls saves calls to C.xxx for later processing.
 func (f *File) saveCall(call *ast.CallExpr, context astContext) {
 	sel, ok := call.Fun.(*ast.SelectorExpr)
 	if !ok {
@@ -279,13 +279,9 @@
 	f.Calls = append(f.Calls, c)
 }
 
-// Save composite literals for later processing.
-func (f *File) saveLiteral(lit *ast.CompositeLit, context astContext) {
-	sel, ok := lit.Type.(*ast.SelectorExpr)
-	if !ok {
-		return
-	}
-	if l, ok := sel.X.(*ast.Ident); !ok || l.Name != "C" || len(lit.Elts) == 0 {
+// saveLiteral saves composite literals for later processing.
+func (f *File) saveLiteral(lit *ast.CompositeLit, context astContext, typeOf typeContext) {
+	if len(lit.Elts) == 0 {
 		return
 	}
 	// if it's already in field:value form, no need to edit
@@ -294,7 +290,28 @@
 			return
 		}
 	}
-	c := &Lit{Lit: lit}
+
+	if lit.Type != nil {
+		// there is an explicit type, hooray.
+		sel, ok := lit.Type.(*ast.SelectorExpr)
+		if !ok {
+			return // Cannot be "C.whatever" // TODO what about "type CdotWhatever C.whatever"?
+		}
+		if l, ok := sel.X.(*ast.Ident); !ok || l.Name != "C" {
+			return
+		}
+		c := &Lit{Lit: lit}
+		f.Lits = append(f.Lits, c)
+		f.LitMap[lit] = c
+		return
+	}
+
+	// otherwise perhaps the type is implicit, e.g. the slice elements in []C.foo{ {1,2}, {3,4}, {5,6} }
+	if typeOf.ty == nil {
+		return
+	}
+	typeOf.path = append([]interface{}{}, typeOf.path...) // must make a private copy
+	c := &Lit{Lit: lit, TypeOf: typeOf}
 	f.Lits = append(f.Lits, c)
 	f.LitMap[lit] = c
 }
@@ -303,7 +320,7 @@
 // This is used when a call has been rewritten, which will also cause
 // the literal to be processed (and it has to be processed as part of the
 // call, otherwise it will cause an "overlapping rewrite" error).
-func (f *File) doneLiteral(x interface{}, context astContext) {
+func (f *File) doneLiteral(x interface{}, context astContext, typeOf typeContext) {
 	if lit, ok := x.(*ast.CompositeLit); ok {
 		if c := f.LitMap[lit]; c != nil {
 			c.Done = true
@@ -312,7 +329,7 @@
 }
 
 // If a function should be exported add it to ExpFunc.
-func (f *File) saveExport(x interface{}, context astContext) {
+func (f *File) saveExport(x interface{}, context astContext, typeOf typeContext) {
 	n, ok := x.(*ast.FuncDecl)
 	if !ok {
 		return
@@ -352,7 +369,7 @@
 }
 
 // Make f.ExpFunc[i] point at the Func from this AST instead of the other one.
-func (f *File) saveExport2(x interface{}, context astContext) {
+func (f *File) saveExport2(x interface{}, context astContext, typeOf typeContext) {
 	n, ok := x.(*ast.FuncDecl)
 	if !ok {
 		return
@@ -388,216 +405,277 @@
 	ctxSelector
 )
 
+// typeContext provides a path into an aggregate type, to later figure out the
+// types of aggregate parts.  These must be entirely syntactic since there's no
+// reliable type information at parsing time (and it's not great at generation time).
+// e.g.,
+//
+//	for T{x, y} the type context for the Compound literal is {T,empty}
+//	for T{{a,b}, {c,d}} the inner type contexts are {T,{0}} and {T,{1}} -- T could be slice or stuct
+//	for T{x:{a,b}, y:{c,d}}, x and y identifiers, the inner type contexts for {a,b} and {c,d} are {T,{"x"} and {T,{"y"}} -- T could be map or struct
+//	for T{x:{a,b}, y:{c,d}}, x and y NOT identifiers, then T must be a map, and
+//	         the inner type contexts for {a,b} and {c,d} are {T,{1} and {T,{1}};
+//	for T{{u,v}:{a,b}, {x,y}:{c,d} }
+//	         the inner type contexts for {u,v} and {x,y} are {T,{0}} and {T,{0}}
+//
+// When no type is provided (as is the case for the inner compound literals above)
+// the existing context is extended in the specified way.
+//
+// When interpreting typeContexts, given types:
+// if T is struct, then integers give positions, strings gave field names
+// if T is slice, integers mean the element type
+// if T is map, 0 means key type, not zero means value type.
+type typeContext struct {
+	ty   ast.Expr
+	path []interface{} // string or integer.
+}
+
+var nilTC typeContext
+
 // walk walks the AST x, calling visit(f, x, context) for each node.
-func (f *File) walk(x interface{}, context astContext, visit func(*File, interface{}, astContext)) {
-	visit(f, x, context)
+func (f *File) walk(x interface{}, context astContext, typeOf typeContext, visit func(*File, interface{}, astContext, typeContext)) {
+	visit(f, x, context, typeOf)
 	switch n := x.(type) {
 	case *ast.Expr:
-		f.walk(*n, context, visit)
+		f.walk(*n, context, typeOf, visit)
 
 	// everything else just recurs
 	default:
-		f.walkUnexpected(x, context, visit)
+		f.walkUnexpected(x, context, typeOf, visit)
 
 	case nil:
 
 	// These are ordered and grouped to match ../../go/ast/ast.go
 	case *ast.Field:
 		if len(n.Names) == 0 && context == ctxField {
-			f.walk(&n.Type, ctxEmbedType, visit)
+			f.walk(&n.Type, ctxEmbedType, typeOf, visit)
 		} else {
-			f.walk(&n.Type, ctxType, visit)
+			f.walk(&n.Type, ctxType, typeOf, visit)
 		}
 	case *ast.FieldList:
 		for _, field := range n.List {
-			f.walk(field, context, visit)
+			f.walk(field, context, typeOf, visit)
 		}
 	case *ast.BadExpr:
 	case *ast.Ident:
 	case *ast.Ellipsis:
-		f.walk(&n.Elt, ctxType, visit)
+		f.walk(&n.Elt, ctxType, typeOf, visit)
 	case *ast.BasicLit:
 	case *ast.FuncLit:
-		f.walk(n.Type, ctxType, visit)
-		f.walk(n.Body, ctxStmt, visit)
+		f.walk(n.Type, ctxType, typeOf, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
 	case *ast.CompositeLit:
-		f.walk(&n.Type, ctxType, visit)
-		f.walk(n.Elts, ctxExpr, visit)
+		f.walk(&n.Type, ctxType, typeOf, visit)
+
+		if n.Type != nil {
+			typeOf = typeContext{ty: n.Type, path: nil}
+		}
+		l := len(typeOf.path)
+		// open-code the elts visit to get the typeOf context right
+		for i, e := range n.Elts {
+			if e == nil {
+				continue
+			}
+
+			if _, ok := e.(*ast.KeyValueExpr); !ok {
+				// KeyValue does its own extension
+				typeOf.path = append(typeOf.path, i)
+			}
+
+			f.walk(&n.Elts[i], context, typeOf, visit)
+			typeOf.path = typeOf.path[0:l]
+		}
+
 	case *ast.ParenExpr:
-		f.walk(&n.X, context, visit)
+		f.walk(&n.X, context, typeOf, visit)
 	case *ast.SelectorExpr:
-		f.walk(&n.X, ctxSelector, visit)
+		f.walk(&n.X, ctxSelector, typeOf, visit)
 	case *ast.IndexExpr:
-		f.walk(&n.X, ctxExpr, visit)
-		f.walk(&n.Index, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
+		f.walk(&n.Index, ctxExpr, typeOf, visit)
 	case *ast.SliceExpr:
-		f.walk(&n.X, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
 		if n.Low != nil {
-			f.walk(&n.Low, ctxExpr, visit)
+			f.walk(&n.Low, ctxExpr, typeOf, visit)
 		}
 		if n.High != nil {
-			f.walk(&n.High, ctxExpr, visit)
+			f.walk(&n.High, ctxExpr, typeOf, visit)
 		}
 		if n.Max != nil {
-			f.walk(&n.Max, ctxExpr, visit)
+			f.walk(&n.Max, ctxExpr, typeOf, visit)
 		}
 	case *ast.TypeAssertExpr:
-		f.walk(&n.X, ctxExpr, visit)
-		f.walk(&n.Type, ctxType, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
+		f.walk(&n.Type, ctxType, typeOf, visit)
 	case *ast.CallExpr:
 		if context == ctxAssign2 {
-			f.walk(&n.Fun, ctxCall2, visit)
+			f.walk(&n.Fun, ctxCall2, typeOf, visit)
 		} else {
-			f.walk(&n.Fun, ctxCall, visit)
+			f.walk(&n.Fun, ctxCall, typeOf, visit)
 		}
-		f.walk(n.Args, ctxExpr, visit)
+		f.walk(n.Args, ctxExpr, typeOf, visit)
 	case *ast.StarExpr:
-		f.walk(&n.X, context, visit)
+		f.walk(&n.X, context, typeOf, visit)
 	case *ast.UnaryExpr:
-		f.walk(&n.X, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
 	case *ast.BinaryExpr:
-		f.walk(&n.X, ctxExpr, visit)
-		f.walk(&n.Y, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
+		f.walk(&n.Y, ctxExpr, typeOf, visit)
 	case *ast.KeyValueExpr:
-		f.walk(&n.Key, ctxExpr, visit)
-		f.walk(&n.Value, ctxExpr, visit)
+
+		if typeOf.ty != nil {
+			l := len(typeOf.path)
+			typeOf.path = append(typeOf.path, 0)
+			f.walk(&n.Key, ctxExpr, typeOf, visit)
+
+			if ident, ok := n.Key.(*ast.Ident); ok {
+				typeOf.path[l] = ident.Name
+			} else {
+				typeOf.path[l] = 1
+			}
+			f.walk(&n.Value, ctxExpr, typeOf, visit)
+			typeOf.path = typeOf.path[0:l]
+		} else {
+			f.walk(&n.Key, ctxExpr, typeOf, visit)
+			f.walk(&n.Value, ctxExpr, typeOf, visit)
+		}
 
 	case *ast.ArrayType:
-		f.walk(&n.Len, ctxExpr, visit)
-		f.walk(&n.Elt, ctxType, visit)
+		f.walk(&n.Len, ctxExpr, typeOf, visit)
+		f.walk(&n.Elt, ctxType, typeOf, visit)
 	case *ast.StructType:
-		f.walk(n.Fields, ctxField, visit)
+		f.walk(n.Fields, ctxField, typeOf, visit)
 	case *ast.FuncType:
 		if tparams := funcTypeTypeParams(n); tparams != nil {
-			f.walk(tparams, ctxParam, visit)
+			f.walk(tparams, ctxParam, typeOf, visit)
 		}
-		f.walk(n.Params, ctxParam, visit)
+		f.walk(n.Params, ctxParam, typeOf, visit)
 		if n.Results != nil {
-			f.walk(n.Results, ctxParam, visit)
+			f.walk(n.Results, ctxParam, typeOf, visit)
 		}
 	case *ast.InterfaceType:
-		f.walk(n.Methods, ctxField, visit)
+		f.walk(n.Methods, ctxField, typeOf, visit)
 	case *ast.MapType:
-		f.walk(&n.Key, ctxType, visit)
-		f.walk(&n.Value, ctxType, visit)
+		f.walk(&n.Key, ctxType, typeOf, visit)
+		f.walk(&n.Value, ctxType, typeOf, visit)
 	case *ast.ChanType:
-		f.walk(&n.Value, ctxType, visit)
+		f.walk(&n.Value, ctxType, typeOf, visit)
 
 	case *ast.BadStmt:
 	case *ast.DeclStmt:
-		f.walk(n.Decl, ctxDecl, visit)
+		f.walk(n.Decl, ctxDecl, typeOf, visit)
 	case *ast.EmptyStmt:
 	case *ast.LabeledStmt:
-		f.walk(n.Stmt, ctxStmt, visit)
+		f.walk(n.Stmt, ctxStmt, typeOf, visit)
 	case *ast.ExprStmt:
-		f.walk(&n.X, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
 	case *ast.SendStmt:
-		f.walk(&n.Chan, ctxExpr, visit)
-		f.walk(&n.Value, ctxExpr, visit)
+		f.walk(&n.Chan, ctxExpr, typeOf, visit)
+		f.walk(&n.Value, ctxExpr, typeOf, visit)
 	case *ast.IncDecStmt:
-		f.walk(&n.X, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
 	case *ast.AssignStmt:
-		f.walk(n.Lhs, ctxExpr, visit)
+		f.walk(n.Lhs, ctxExpr, typeOf, visit)
 		if len(n.Lhs) == 2 && len(n.Rhs) == 1 {
-			f.walk(n.Rhs, ctxAssign2, visit)
+			f.walk(n.Rhs, ctxAssign2, typeOf, visit)
 		} else {
-			f.walk(n.Rhs, ctxExpr, visit)
+			f.walk(n.Rhs, ctxExpr, typeOf, visit)
 		}
 	case *ast.GoStmt:
-		f.walk(n.Call, ctxExpr, visit)
+		f.walk(n.Call, ctxExpr, typeOf, visit)
 	case *ast.DeferStmt:
-		f.walk(n.Call, ctxDefer, visit)
+		f.walk(n.Call, ctxDefer, typeOf, visit)
 	case *ast.ReturnStmt:
-		f.walk(n.Results, ctxExpr, visit)
+		f.walk(n.Results, ctxExpr, typeOf, visit)
 	case *ast.BranchStmt:
 	case *ast.BlockStmt:
-		f.walk(n.List, context, visit)
+		f.walk(n.List, context, typeOf, visit)
 	case *ast.IfStmt:
-		f.walk(n.Init, ctxStmt, visit)
-		f.walk(&n.Cond, ctxExpr, visit)
-		f.walk(n.Body, ctxStmt, visit)
-		f.walk(n.Else, ctxStmt, visit)
+		f.walk(n.Init, ctxStmt, typeOf, visit)
+		f.walk(&n.Cond, ctxExpr, typeOf, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
+		f.walk(n.Else, ctxStmt, typeOf, visit)
 	case *ast.CaseClause:
 		if context == ctxTypeSwitch {
 			context = ctxType
 		} else {
 			context = ctxExpr
 		}
-		f.walk(n.List, context, visit)
-		f.walk(n.Body, ctxStmt, visit)
+		f.walk(n.List, context, typeOf, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
 	case *ast.SwitchStmt:
-		f.walk(n.Init, ctxStmt, visit)
-		f.walk(&n.Tag, ctxExpr, visit)
-		f.walk(n.Body, ctxSwitch, visit)
+		f.walk(n.Init, ctxStmt, typeOf, visit)
+		f.walk(&n.Tag, ctxExpr, typeOf, visit)
+		f.walk(n.Body, ctxSwitch, typeOf, visit)
 	case *ast.TypeSwitchStmt:
-		f.walk(n.Init, ctxStmt, visit)
-		f.walk(n.Assign, ctxStmt, visit)
-		f.walk(n.Body, ctxTypeSwitch, visit)
+		f.walk(n.Init, ctxStmt, typeOf, visit)
+		f.walk(n.Assign, ctxStmt, typeOf, visit)
+		f.walk(n.Body, ctxTypeSwitch, typeOf, visit)
 	case *ast.CommClause:
-		f.walk(n.Comm, ctxStmt, visit)
-		f.walk(n.Body, ctxStmt, visit)
+		f.walk(n.Comm, ctxStmt, typeOf, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
 	case *ast.SelectStmt:
-		f.walk(n.Body, ctxStmt, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
 	case *ast.ForStmt:
-		f.walk(n.Init, ctxStmt, visit)
-		f.walk(&n.Cond, ctxExpr, visit)
-		f.walk(n.Post, ctxStmt, visit)
-		f.walk(n.Body, ctxStmt, visit)
+		f.walk(n.Init, ctxStmt, typeOf, visit)
+		f.walk(&n.Cond, ctxExpr, typeOf, visit)
+		f.walk(n.Post, ctxStmt, typeOf, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
 	case *ast.RangeStmt:
-		f.walk(&n.Key, ctxExpr, visit)
-		f.walk(&n.Value, ctxExpr, visit)
-		f.walk(&n.X, ctxExpr, visit)
-		f.walk(n.Body, ctxStmt, visit)
+		f.walk(&n.Key, ctxExpr, typeOf, visit)
+		f.walk(&n.Value, ctxExpr, typeOf, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
+		f.walk(n.Body, ctxStmt, typeOf, visit)
 
 	case *ast.ImportSpec:
 	case *ast.ValueSpec:
-		f.walk(&n.Type, ctxType, visit)
+		f.walk(&n.Type, ctxType, typeOf, visit)
 		if len(n.Names) == 2 && len(n.Values) == 1 {
-			f.walk(&n.Values[0], ctxAssign2, visit)
+			f.walk(&n.Values[0], ctxAssign2, typeOf, visit)
 		} else {
-			f.walk(n.Values, ctxExpr, visit)
+			f.walk(n.Values, ctxExpr, typeOf, visit)
 		}
 	case *ast.TypeSpec:
 		if tparams := typeSpecTypeParams(n); tparams != nil {
-			f.walk(tparams, ctxParam, visit)
+			f.walk(tparams, ctxParam, typeOf, visit)
 		}
-		f.walk(&n.Type, ctxType, visit)
+		f.walk(&n.Type, ctxType, typeOf, visit)
 
 	case *ast.BadDecl:
 	case *ast.GenDecl:
-		f.walk(n.Specs, ctxSpec, visit)
+		f.walk(n.Specs, ctxSpec, typeOf, visit)
 	case *ast.FuncDecl:
 		if n.Recv != nil {
-			f.walk(n.Recv, ctxParam, visit)
+			f.walk(n.Recv, ctxParam, typeOf, visit)
 		}
-		f.walk(n.Type, ctxType, visit)
+		f.walk(n.Type, ctxType, typeOf, visit)
 		if n.Body != nil {
-			f.walk(n.Body, ctxStmt, visit)
+			f.walk(n.Body, ctxStmt, typeOf, visit)
 		}
 
 	case *ast.File:
-		f.walk(n.Decls, ctxDecl, visit)
+		f.walk(n.Decls, ctxDecl, typeOf, visit)
 
 	case *ast.Package:
 		for _, file := range n.Files {
-			f.walk(file, ctxFile, visit)
+			f.walk(file, ctxFile, typeOf, visit)
 		}
 
 	case []ast.Decl:
 		for _, d := range n {
-			f.walk(d, context, visit)
+			f.walk(d, context, typeOf, visit)
 		}
 	case []ast.Expr:
 		for i := range n {
-			f.walk(&n[i], context, visit)
+			f.walk(&n[i], context, typeOf, visit)
 		}
 	case []ast.Stmt:
 		for _, s := range n {
-			f.walk(s, context, visit)
+			f.walk(s, context, typeOf, visit)
 		}
 	case []ast.Spec:
 		for _, s := range n {
-			f.walk(s, context, visit)
+			f.walk(s, context, typeOf, visit)
 		}
 	}
 }
diff --git a/src/cmd/cgo/ast_go1.go b/src/cmd/cgo/ast_go1.go
index 2f65f0f..59ec7d5 100644
--- a/src/cmd/cgo/ast_go1.go
+++ b/src/cmd/cgo/ast_go1.go
@@ -11,7 +11,7 @@
 	"go/token"
 )
 
-func (f *File) walkUnexpected(x interface{}, context astContext, visit func(*File, interface{}, astContext)) {
+func (f *File) walkUnexpected(x interface{}, context astContext, typeOf typeContext, visit func(*File, interface{}, astContext, typeContext)) {
 	error_(token.NoPos, "unexpected type %T in walk", x)
 	panic("unexpected type")
 }
diff --git a/src/cmd/cgo/ast_go118.go b/src/cmd/cgo/ast_go118.go
index ced3072..f43b2d0 100644
--- a/src/cmd/cgo/ast_go118.go
+++ b/src/cmd/cgo/ast_go118.go
@@ -11,15 +11,15 @@
 	"go/token"
 )
 
-func (f *File) walkUnexpected(x interface{}, context astContext, visit func(*File, interface{}, astContext)) {
+func (f *File) walkUnexpected(x interface{}, context astContext, typeOf typeContext, visit func(*File, interface{}, astContext, typeContext)) {
 	switch n := x.(type) {
 	default:
 		error_(token.NoPos, "unexpected type %T in walk", x)
 		panic("unexpected type")
 
 	case *ast.IndexListExpr:
-		f.walk(&n.X, ctxExpr, visit)
-		f.walk(n.Indices, ctxExpr, visit)
+		f.walk(&n.X, ctxExpr, typeOf, visit)
+		f.walk(n.Indices, ctxExpr, typeOf, visit)
 	}
 }
 
diff --git a/src/cmd/cgo/gcc.go b/src/cmd/cgo/gcc.go
index 8d8e19d..f95eb50 100644
--- a/src/cmd/cgo/gcc.go
+++ b/src/cmd/cgo/gcc.go
@@ -801,18 +801,99 @@
 	}
 }
 
-func typeFor(cType string) *Type {
-	ty := typedef["_Ctype_struct_"+cType]
-	if ty != nil {
-		return ty
+func typeForExpr(e ast.Expr) *Type {
+	switch x := e.(type) {
+	case *ast.SelectorExpr:
+		if l, ok := x.X.(*ast.Ident); ok && l.Name == "C" {
+			cType := x.Sel.Name
+			if ty := typedef["_Ctype_struct_"+cType]; ty != nil {
+				return ty
+			}
+			return typedef["_Ctype_"+cType]
+		}
+		return nil
+	case *ast.Ident:
+		return typedef[x.Name]
 	}
-	ty = typedef["_Ctype_"+cType]
-	return ty
+	return nil
+}
+
+func typeFor(lit *Lit) *Type {
+	if lit.Lit.Type != nil {
+		sel := lit.Lit.Type.(*ast.SelectorExpr)
+		cType := sel.Sel.Name
+		if ty := typedef["_Ctype_struct_"+cType]; ty != nil {
+			return ty
+		}
+		return typedef["_Ctype_"+cType]
+	}
+	t := lit.TypeOf.ty
+	if t == nil {
+		return nil
+	}
+	p := lit.TypeOf.path
+	for len(p) > 0 {
+		i := -1  // not a valid field index
+		s := ";" // not a valid field name
+		switch x := p[0].(type) {
+		case int:
+			i = x
+		case string:
+			s = x
+		default:
+			return nil
+		}
+
+		switch x := t.(type) {
+		case *ast.ArrayType:
+			t = x.Elt
+
+		// Not actually a case?
+		case *ast.StructType:
+			// Count non-"_" fields looking for some kind of match
+		structLoop:
+			for _, fl := range x.Fields.List {
+				for _, f := range fl.Names {
+					if f.Name == "_" {
+						continue
+					}
+					if f.Name == s || i == 0 {
+						t = fl.Type
+						break structLoop
+					}
+					i--
+				}
+			}
+
+		case *ast.MapType:
+			if i == 0 {
+				t = x.Key
+			} else {
+				t = x.Value
+			}
+		case *ast.Ident, *ast.SelectorExpr:
+			ty := typeForExpr(t)
+			if ty == nil {
+				return nil
+			}
+			t = ty.Go
+			continue // do not consume a path element
+
+		default:
+			return nil
+		}
+		p = p[1:]
+	}
+	return typeForExpr(t)
 }
 
 func (p *Package) prewriteLit(lit *Lit) {
-	sel := lit.Lit.Type.(*ast.SelectorExpr)
-	ty := typeFor(sel.Sel.Name)
+	ty := typeFor(lit)
+	if ty == nil {
+		// interior typeless compound literals could have nothing at all to do with cgo.  Leave them alone.
+		lit.Done = true
+		return
+	}
 	fields := ty.Go.(*ast.StructType).Fields.List
 	j := 0
 	for _, fs := range fields {
@@ -839,8 +920,7 @@
 }
 
 func (p *Package) rewriteLit(f *File, lit *Lit) {
-	sel := lit.Lit.Type.(*ast.SelectorExpr)
-	ty := typeFor(sel.Sel.Name)
+	ty := typeFor(lit)
 	fields := ty.Go.(*ast.StructType).Fields.List
 	j := 0
 	for _, fs := range fields {
@@ -876,7 +956,7 @@
 			if nu {
 				needsUnsafe = true
 			}
-			f.walk(call.Call, ctxExpr, (*File).doneLiteral)
+			f.walk(call.Call, ctxExpr, nilTC, (*File).doneLiteral)
 		}
 	}
 	return needsUnsafe
@@ -1197,7 +1277,7 @@
 // If addPosition is true, add position info to the idents of C names in arg.
 func (p *Package) mangle(f *File, arg *ast.Expr, addPosition bool) (ast.Expr, bool) {
 	needsUnsafe := false
-	f.walk(arg, ctxExpr, func(f *File, arg interface{}, context astContext) {
+	f.walk(arg, ctxExpr, nilTC, func(f *File, arg interface{}, context astContext, typeOf typeContext) {
 		px, ok := arg.(*ast.Expr)
 		if !ok {
 			return
diff --git a/src/cmd/cgo/internal/testerrors/ptr_test.go b/src/cmd/cgo/internal/testerrors/ptr_test.go
index ce434ce..31b620c 100644
--- a/src/cmd/cgo/internal/testerrors/ptr_test.go
+++ b/src/cmd/cgo/internal/testerrors/ptr_test.go
@@ -88,6 +88,32 @@
 		fail:    false,
 	},
 	{
+		// Implicit struct type, within slice of struct.
+		name:    "sliceOfStruct",
+		c:       `struct s6a { int x; int y; }; void f6a(struct s6a *p) {}`,
+		imports: []string{"unsafe"},
+		body:    `s := []C.struct_s6a{{1,2}}; C.f6a(&s[0])`,
+		fail:    false,
+	},
+	{
+		// Implicit struct type, within map of struct
+		name:    "mapOfStruct",
+		c:       `struct s6k { int x; int y; }; struct s6v { int x; int y; int z; }; void f6k(struct s6k *p) {}; void f6v(struct s6v *p) {}`,
+		imports: []string{"unsafe"},
+		body:    `s := map[C.struct_s6k]C.struct_s6v{{1,2}:{3,4,5},{6,7}:{8,9,10}}; for k,v := range s {C.f6k(&k); C.f6v(&v)}`,
+		fail:    false,
+	},
+	{
+		// Implicit struct type, within slice of map of struct
+		// Also includes a "false positive" untyped compound literal that should be left alone.
+		name:    "sliceOfmapOfStruct",
+		c:       `struct s6sk { int x; int y; }; struct s6sv { int x; int y; int z; }; void f6sk(struct s6sk *p) {}; void f6sv(struct s6sv *p) {}`,
+		imports: []string{"unsafe"},
+		body:    `ss:= [][]C.int{{1, 2}, {3, 4}}; s := []map[C.struct_s6sk]C.struct_s6sv{{{ss[0][0],2}:{3,4,5},{6,7}:{8,9,10}}}; for k,v := range s[0] {C.f6sk(&k); C.f6sv(&v)}`,
+		fail:    false,
+	},
+
+	{
 		// Passing the address of a slice with a Go pointer.
 		name:    "sliceptr1",
 		c:       `void f7(void **p) {}`,
diff --git a/src/cmd/cgo/main.go b/src/cmd/cgo/main.go
index 3aad849..e97635c 100644
--- a/src/cmd/cgo/main.go
+++ b/src/cmd/cgo/main.go
@@ -102,8 +102,9 @@
 
 // A Lit refers to a composite literal of a C.xxx type in the AST.
 type Lit struct {
-	Lit  *ast.CompositeLit
-	Done bool
+	Lit    *ast.CompositeLit
+	TypeOf typeContext
+	Done   bool
 }
 
 // A Ref refers to an expression of the form C.xxx in the AST.

Change information

Files:

• M src/cmd/cgo/ast.go
• M src/cmd/cgo/ast_go1.go
• M src/cmd/cgo/ast_go118.go
• M src/cmd/cgo/gcc.go
• M src/cmd/cgo/internal/testerrors/ptr_test.go
• M src/cmd/cgo/main.go

Change size: L

Delta: 6 files changed, 314 insertions(+), 129 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newchange

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 1

Gerrit-Owner: David Chase <drc...@google.com>

[David Chase (Gerrit)]

David Chase uploaded new patchset

David Chase uploaded patch set #2 to this change.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 2

Gerrit-Owner: David Chase <drc...@google.com>

[David Chase (Gerrit)]

David Chase voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 2

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Comment-Date: Mon, 24 Feb 2025 22:51:09 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[David Chase (Gerrit)]

David Chase uploaded new patchset

David Chase uploaded patch set #3 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 3

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

[David Chase (Gerrit)]

David Chase voted Commit-Queue+1

Commit-Queue

+1

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 3

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Comment-Date: Mon, 24 Mar 2025 05:11:19 +0000

Gerrit-HasComments: No

Gerrit-Has-Labels: Yes

[Ian Lance Taylor (Gerrit)]

Attention needed from David Chase

Ian Lance Taylor added 6 comments

File src/cmd/cgo/ast.go

Line 313, Patchset 6 (Latest): typeOf.path = append([]interface{}{}, typeOf.path...) // must make a private copy

Ian Lance Taylor . unresolved

Since the bootstrap version is 1.24, this can be

typeOf.path = slices.Clone(typeOf.path)

I think it would be helpful if the comment said _why_ we need a private copy.

Line 416, Patchset 6 (Latest):// for T{x:{a,b}, y:{c,d}}, x and y NOT identifiers, then T must be a map, and

Ian Lance Taylor . unresolved

It's not obvious that this case must be a map, it seems like it could be an array with numeric indexes as in [2]S{0:{a,b}, 1:{c, d}}.

Similarly the line above could be an array if x and y are named consts.

Line 426, Patchset 6 (Latest):// if T is slice, integers mean the element type

Ian Lance Taylor . unresolved

or array

Line 430, Patchset 6 (Latest): path []interface{} // string or integer.

Ian Lance Taylor . unresolved

s/interface{}/any/

Line 433, Patchset 6 (Latest):var nilTC typeContext

Ian Lance Taylor . unresolved

Add a doc comment.

File src/cmd/cgo/gcc.go

Line 876, Patchset 6 (Latest): return nil

Ian Lance Taylor . unresolved

This case seems like it should fatal out, it looks like an internal error.

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 6

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: David Chase <drc...@google.com>

Gerrit-Comment-Date: Fri, 31 Oct 2025 00:01:56 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[David Chase (Gerrit)]

Attention needed from David Chase

David Chase uploaded new patchset

David Chase uploaded patch set #7 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 7

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase added 6 comments

File src/cmd/cgo/ast.go

Line 313, Patchset 6: typeOf.path = append([]interface{}{}, typeOf.path...) // must make a private copy

Ian Lance Taylor . unresolved

Since the bootstrap version is 1.24, this can be

typeOf.path = slices.Clone(typeOf.path)

I think it would be helpful if the comment said _why_ we need a private copy.

David Chase

Half-done. I am trying to remember why that private copy was necessary.

Line 416, Patchset 6:// for T{x:{a,b}, y:{c,d}}, x and y NOT identifiers, then T must be a map, and

Ian Lance Taylor . resolved

It's not obvious that this case must be a map, it seems like it could be an array with numeric indexes as in [2]S{0:{a,b}, 1:{c, d}}.

Similarly the line above could be an array if x and y are named consts.

David Chase

Done

Line 426, Patchset 6:// if T is slice, integers mean the element type

Ian Lance Taylor . resolved

or array

David Chase

Done

Line 430, Patchset 6: path []interface{} // string or integer.

Ian Lance Taylor . resolved

s/interface{}/any/

David Chase

Done

Line 433, Patchset 6:var nilTC typeContext

Ian Lance Taylor . resolved

Add a doc comment.

David Chase

Done

File src/cmd/cgo/gcc.go

Line 876, Patchset 6: return nil

Ian Lance Taylor . resolved

This case seems like it should fatal out, it looks like an internal error.

David Chase

Done

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 7

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Wed, 12 Nov 2025 23:21:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: Ian Lance Taylor <ia...@golang.org>

[Ian Lance Taylor (Gerrit)]

Attention needed from David Chase

Ian Lance Taylor added 1 comment

File src/cmd/cgo/ast.go

Line 294, Patchset 7 (Latest): }

Ian Lance Taylor . unresolved

I think we can break here, if the first is not a KeyValueExpr then none of them should be KeyValueExprs. In fact I think we don't need a range, we can just test the first element.

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 7

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: David Chase <drc...@google.com>

Gerrit-Comment-Date: Sun, 16 Nov 2025 04:29:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[David Chase (Gerrit)]

Attention needed from David Chase

David Chase uploaded new patchset

David Chase uploaded patch set #8 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 8

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase added 2 comments

File src/cmd/cgo/ast.go

Line 294, Patchset 7: }

Ian Lance Taylor . resolved

I think we can break here, if the first is not a KeyValueExpr then none of them should be KeyValueExprs. In fact I think we don't need a range, we can just test the first element.

David Chase

Done

Line 313, Patchset 6: typeOf.path = append([]interface{}{}, typeOf.path...) // must make a private copy

Ian Lance Taylor . resolved

Since the bootstrap version is 1.24, this can be

typeOf.path = slices.Clone(typeOf.path)

I think it would be helpful if the comment said _why_ we need a private copy.

David Chase

Half-done. I am trying to remember why that private copy was necessary.

David Chase

Done

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 8

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Mon, 24 Nov 2025 17:47:31 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: David Chase <drc...@google.com>

[Ian Lance Taylor (Gerrit)]

Attention needed from David Chase

Ian Lance Taylor added 6 comments

File src/cmd/cgo/ast.go

Line 417, Patchset 8 (Latest):// for T{{a,b}, {c,d}} the inner type contexts are {T,{0}} and {T,{1}} -- T could be slice or stuct

Ian Lance Taylor . unresolved

s/stuct/struct/

Line 482, Patchset 8 (Latest): l := len(typeOf.path)

Ian Lance Taylor . unresolved

Let's not use the name l which looks so much like 1 and make the slice below [0:l] easily confused with [0:1]. ln would be a fine name here.

Line 490, Patchset 8 (Latest): // KeyValue does its own extension

Ian Lance Taylor . unresolved

I think this comment needs to be more explicit.

// Note that if e is a KeyValueExpr, we will append an element to path in the KeyValueExpr case below.

But now that I write that, I don't understand this. The KeyValueExpr case appends to path and also trims path back to what it was. Here for the non-KeyValueExpr case we append to path. But then either way we trim path back.

I guess it works OK because we don't remove the last element, we just go back to the length we had before. So the fact that we don't do that twice in the KeyValueExpr case doesn't hurt anything.

But I can't help feel that it would be simpler if we always append here and always trim here, and we don't do either in the KeyValueExpr case.

Line 539, Patchset 8 (Latest): l := len(typeOf.path)

Ian Lance Taylor . unresolved

Use a name other than l.

File src/cmd/cgo/gcc.go

Line 876, Patchset 8 (Latest): panic(fmt.Errorf("Internal error, expected int or string but got a %T (%v)", p[0], p[0]))

Ian Lance Taylor . unresolved

Just call fatalf.

Line 890, Patchset 8 (Latest): continue

Ian Lance Taylor . unresolved

Why is this correct? When a struct composite literal doesn't use names, it has to give a value for an underscore key. I'm sure I'm forgetting some of the details of this overall change, but that is how it works today. So it seems like we should decrement i in this case.

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 8

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: David Chase <drc...@google.com>

Gerrit-Comment-Date: Mon, 24 Nov 2025 20:16:42 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[David Chase (Gerrit)]

Attention needed from David Chase

David Chase uploaded new patchset

David Chase uploaded patch set #9 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 9

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase added 7 comments

Patchset-level comments

File-level comment, Patchset 8:

David Chase . resolved

There will be another CL with improved comments.

File src/cmd/cgo/ast.go

Line 417, Patchset 8:// for T{{a,b}, {c,d}} the inner type contexts are {T,{0}} and {T,{1}} -- T could be slice or stuct

Ian Lance Taylor . resolved

s/stuct/struct/

David Chase

Done

Line 482, Patchset 8: l := len(typeOf.path)

Ian Lance Taylor . resolved

Let's not use the name l which looks so much like 1 and make the slice below [0:l] easily confused with [0:1]. ln would be a fine name here.

David Chase

Done

Line 490, Patchset 8: // KeyValue does its own extension

Ian Lance Taylor . unresolved

I think this comment needs to be more explicit.

// Note that if e is a KeyValueExpr, we will append an element to path in the KeyValueExpr case below.

But now that I write that, I don't understand this. The KeyValueExpr case appends to path and also trims path back to what it was. Here for the non-KeyValueExpr case we append to path. But then either way we trim path back.

I guess it works OK because we don't remove the last element, we just go back to the length we had before. So the fact that we don't do that twice in the KeyValueExpr case doesn't hurt anything.

But I can't help feel that it would be simpler if we always append here and always trim here, and we don't do either in the KeyValueExpr case.

David Chase

KeyValue does bespoke appending, and also modifies the append element for its two subtree walks. Arguably the first append could go here, it feels a bit like pushing the ugly lump around under a rug. I added a comment explaining this. (resolved?)

Line 539, Patchset 8: l := len(typeOf.path)

Ian Lance Taylor . resolved

Use a name other than l.

David Chase

Done

File src/cmd/cgo/gcc.go

Line 876, Patchset 8: panic(fmt.Errorf("Internal error, expected int or string but got a %T (%v)", p[0], p[0]))

Ian Lance Taylor . resolved

Just call fatalf.

David Chase

Done

Line 890, Patchset 8: continue

Ian Lance Taylor . resolved

Why is this correct? When a struct composite literal doesn't use names, it has to give a value for an underscore key. I'm sure I'm forgetting some of the details of this overall change, but that is how it works today. So it seems like we should decrement i in this case.

David Chase

It wasn't correct, now it is. I added a test including a C struct containing a "\_" field, which broke it, I modified it to rename all but the first "\_" field to the cgo padding name. This needs comments, but thought I should stop here and see what you think of this approach.

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 8

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Mon, 24 Nov 2025 22:37:06 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase added 1 comment

Patchset-level comments

File-level comment, Patchset 9 (Latest):

David Chase . resolved

I either need to combine this CL with the next one, or move pieces of it into the next one.

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 9

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Tue, 25 Nov 2025 14:34:27 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase uploaded new patchset

David Chase uploaded patch set #10 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result-1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 10

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase added 1 comment

File src/cmd/cgo/ast.go

Line 490, Patchset 8: // KeyValue does its own extension

Ian Lance Taylor . resolved

I think this comment needs to be more explicit.

// Note that if e is a KeyValueExpr, we will append an element to path in the KeyValueExpr case below.

But now that I write that, I don't understand this. The KeyValueExpr case appends to path and also trims path back to what it was. Here for the non-KeyValueExpr case we append to path. But then either way we trim path back.

I guess it works OK because we don't remove the last element, we just go back to the length we had before. So the fact that we don't do that twice in the KeyValueExpr case doesn't hurt anything.

But I can't help feel that it would be simpler if we always append here and always trim here, and we don't do either in the KeyValueExpr case.

David Chase

KeyValue does bespoke appending, and also modifies the append element for its two subtree walks. Arguably the first append could go here, it feels a bit like pushing the ugly lump around under a rug. I added a comment explaining this. (resolved?)

David Chase

Marked as resolved.

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 10

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Tue, 25 Nov 2025 21:24:41 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

Comment-In-Reply-To: David Chase <drc...@google.com>

[Ian Lance Taylor (Gerrit)]

Attention needed from David Chase

Ian Lance Taylor added 6 comments

File src/cmd/cgo/gcc.go

Line 885, Patchset 10 (Latest): // Not actually a case?

Ian Lance Taylor . unresolved

Not sure what this comment "not actually a case?" means.

Line 887, Patchset 10 (Latest): // Count non-"_" fields looking for some kind of match

Ian Lance Taylor . unresolved

This comment should repeat what it says below, that underscore fields are annotations inserted by the tool.

Line 3472, Patchset 10 (Latest):// allFieldNames returns the names of all the fields in fld separated by spaces

Ian Lance Taylor . unresolved

Add a comment that this is only used for error reporting.

File src/cmd/cgo/internal/test/test.go

Line 2262, Patchset 10 (Latest): p: nil,

Ian Lance Taylor . unresolved

Why add these keys to the composite literals? Seems like the point of this CL is that these keys are unnecessary. If they are needed for some reason, won't that break existing code that is like this code?

File src/cmd/cgo/internal/testerrors/ptr_test.go

Line 96, Patchset 10 (Latest): imports: []string{"unsafe"},

Ian Lance Taylor . unresolved

Why is this test and others below importing "unsafe"? I don't see a use of "unsafe" here.

Line 691, Patchset 10 (Latest): if err != nil {

Ian Lance Taylor . unresolved

Easier to write as

    out, err := cmd.CombinedOutput()
    t.Logf("%s", out)
    if err != nil {
        t.Fatalf("go build: %v", err)
    }

Note that the t.Log does not need a \n at the end.

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 10

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: David Chase <drc...@google.com>

Gerrit-Comment-Date: Wed, 26 Nov 2025 02:06:40 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[David Chase (Gerrit)]

Attention needed from David Chase

David Chase uploaded new patchset

David Chase uploaded patch set #11 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 11

[David Chase (Gerrit)]

Attention needed from David Chase

David Chase uploaded new patchset

David Chase uploaded patch set #12 to this change.

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 12

[David Chase (Gerrit)]

Attention needed from Ian Lance Taylor

David Chase added 6 comments

File src/cmd/cgo/gcc.go

Line 885, Patchset 10: // Not actually a case?

Ian Lance Taylor . resolved

Not sure what this comment "not actually a case?" means.

David Chase

deleted

Line 887, Patchset 10: // Count non-"_" fields looking for some kind of match

Ian Lance Taylor . resolved

This comment should repeat what it says below, that underscore fields are annotations inserted by the tool.

David Chase

Done

Line 3472, Patchset 10:// allFieldNames returns the names of all the fields in fld separated by spaces

Ian Lance Taylor . resolved

Add a comment that this is only used for error reporting.

David Chase

Done

File src/cmd/cgo/internal/test/test.go

Line 2262, Patchset 10: p: nil,

Ian Lance Taylor . resolved

Why add these keys to the composite literals? Seems like the point of this CL is that these keys are unnecessary. If they are needed for some reason, won't that break existing code that is like this code?

David Chase

Fixed. Typedefs weren't fully handled. This also uncovered a bug in srcimporter -- it was importing go code before cgo processing, not after.

File src/cmd/cgo/internal/testerrors/ptr_test.go

Line 96, Patchset 10: imports: []string{"unsafe"},

Ian Lance Taylor . resolved

Why is this test and others below importing "unsafe"? I don't see a use of "unsafe" here.

David Chase

Done (I think -- may not have gotten every last one)

Line 691, Patchset 10: if err != nil {

Ian Lance Taylor . resolved

Easier to write as

    out, err := cmd.CombinedOutput()
    t.Logf("%s", out)
    if err != nil {
        t.Fatalf("go build: %v", err)
    }

Note that the t.Log does not need a \n at the end.

David Chase

Done

Open in Gerrit

Related details

Attention is currently required from:

• Ian Lance Taylor

Submit Requirements:

• Code-Review

• No-Unresolved-Comments

• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 12

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Wed, 26 Nov 2025 21:14:03 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: No

[Ian Lance Taylor (Gerrit)]

Attention needed from David Chase

Ian Lance Taylor voted and added 2 comments

Votes added by Ian Lance Taylor

Code-Review

+2

2 comments

Patchset-level comments

File-level comment, Patchset 12 (Latest):

Ian Lance Taylor . resolved

The cgo part looks good now. Not really sure about the srcimporter part.

File src/go/internal/srcimporter/srcimporter.go

Line 248, Patchset 12 (Latest): panic(err)

Ian Lance Taylor . unresolved

Seems like this should return this error rather than panic.

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments

• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 12

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Attention: David Chase <drc...@google.com>

Gerrit-Comment-Date: Thu, 27 Nov 2025 04:28:36 +0000

Gerrit-HasComments: Yes

Gerrit-Has-Labels: Yes

[David Chase (Gerrit)]

Attention needed from David Chase

David Chase uploaded new patchset

David Chase uploaded patch set #13 to this change.

Following approvals got outdated and were removed:

• TryBots-Pass: LUCI-TryBot-Result+1 by Go LUCI

Open in Gerrit

Related details

Attention is currently required from:

• David Chase

Submit Requirements:

• Code-Review
• No-Unresolved-Comments
• Review-Enforcement

• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: newpatchset

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 13

[David Chase (Gerrit)]

David Chase added 2 comments

Patchset-level comments

File-level comment, Patchset 13 (Latest):

David Chase . resolved

Thanks for the review help. Sorry it was so last minute.

File src/go/internal/srcimporter/srcimporter.go

Line 248, Patchset 12: panic(err)

Ian Lance Taylor . resolved

Seems like this should return this error rather than panic.

David Chase

done, along with other error cleanups.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

• Code-Review
• No-Unresolved-Comments

• Review-Enforcement
• TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

Gerrit-MessageType: comment

Gerrit-Project: go

Gerrit-Branch: master

Gerrit-Change-Id: I8b662689f0dac7f3d6828e9c27ff291cf647668b

Gerrit-Change-Number: 647400

Gerrit-PatchSet: 13

Gerrit-Owner: David Chase <drc...@google.com>

Gerrit-Reviewer: David Chase <drc...@google.com>

Gerrit-Reviewer: Go LUCI <golang...@luci-project-accounts.iam.gserviceaccount.com>

Gerrit-Reviewer: Ian Lance Taylor <ia...@golang.org>

Gerrit-Comment-Date: Thu, 27 Nov 2025 04:50:16 +0000