From: Robert Griesemer Date: Tue, 25 Sep 2018 22:31:23 +0000 (-0700) Subject: go/internal/gccgoimporter: fix updating of "forward declared" types X-Git-Tag: go1.12beta1~993 X-Git-Url: http://www.git.cypherpunks.su/?a=commitdiff_plain;h=9c81402f58ae83987f32153c1587c9f03b4a5769;p=gostls13.git go/internal/gccgoimporter: fix updating of "forward declared" types The existing code uses a type map which associates a type number with a type; references to existing types are expressed via the type number in the export data. Before this CL, type map entries were set when a type was read in completely, which meant that recursive references to types (i.e., type map entries) that were in the middle of construction (i.e., where the type map was not yet updated) would lead to nil types. Such cycles are usually created via defined types which introduce a types.Named entry into the type map before the underlying type is parsed; in this case the code worked. In case of type aliases, no such "forwarder" exists and type cycles lead to nil types. This CL fixes the problem by a) updating the type map as soon as a type becomes available but before the type's components are parsed; b) keeping track of a list of type map entries that may need to be updated together (because of aliases that may all refer to the same type); and c) adding (redundant) markers to the type map to detect algorithmic errors. Also: - distinguish between parseInt and parseInt64 - added more test cases Fixes #27856. Change-Id: Iba701439ea3231aa435b7b80ea2d419db2af3be1 Reviewed-on: https://go-review.googlesource.com/137857 Run-TryBot: Robert Griesemer TryBot-Result: Gobot Gobot Reviewed-by: Ian Lance Taylor --- diff --git a/src/go/internal/gccgoimporter/importer_test.go b/src/go/internal/gccgoimporter/importer_test.go index 5a699687bd..15494fd6b3 100644 --- a/src/go/internal/gccgoimporter/importer_test.go +++ b/src/go/internal/gccgoimporter/importer_test.go @@ -102,8 +102,10 @@ var importerTests = [...]importerTest{ {pkgpath: "unicode", name: "MaxRune", want: "const MaxRune untyped rune", wantval: "1114111"}, {pkgpath: "imports", wantinits: []string{"imports..import", "fmt..import", "math..import"}}, {pkgpath: "importsar", name: "Hello", want: "var Hello string"}, - {pkgpath: "alias", name: "IntAlias2", want: "type IntAlias2 = Int"}, + {pkgpath: "aliases", name: "A14", want: "type A14 = func(int, T0) chan T2"}, + {pkgpath: "aliases", name: "C0", want: "type C0 struct{f1 C1; f2 C1}"}, {pkgpath: "escapeinfo", name: "NewT", want: "func NewT(data []byte) *T"}, + {pkgpath: "issue27856", name: "M", want: "type M struct{E F}"}, } func TestGoxImporter(t *testing.T) { diff --git a/src/go/internal/gccgoimporter/parser.go b/src/go/internal/gccgoimporter/parser.go index 9f8c19b638..f64be54d66 100644 --- a/src/go/internal/gccgoimporter/parser.go +++ b/src/go/internal/gccgoimporter/parser.go @@ -378,52 +378,80 @@ func (p *parser) parseConst(pkg *types.Package) *types.Const { return types.NewConst(token.NoPos, pkg, name, typ, val) } +// reserved is a singleton type used to fill type map slots that have +// been reserved (i.e., for which a type number has been parsed) but +// which don't have their actual type yet. When the type map is updated, +// the actual type must replace a reserved entry (or we have an internal +// error). Used for self-verification only - not required for correctness. +var reserved = new(struct{ types.Type }) + +// reserve reserves the type map entry n for future use. +func (p *parser) reserve(n int) { + if p.typeMap[n] != nil { + p.errorf("internal error: type %d already used", n) + } + p.typeMap[n] = reserved +} + +// update sets the type map entries for the given type numbers nlist to t. +func (p *parser) update(t types.Type, nlist []int) { + for _, n := range nlist { + if p.typeMap[n] != reserved { + p.errorf("internal error: typeMap[%d] not reserved", n) + } + p.typeMap[n] = t + } +} + // NamedType = TypeName [ "=" ] Type { Method } . // TypeName = ExportedName . // Method = "func" "(" Param ")" Name ParamList ResultList ";" . -func (p *parser) parseNamedType(n int) types.Type { +func (p *parser) parseNamedType(nlist []int) types.Type { pkg, name := p.parseExportedName() scope := pkg.Scope() + obj := scope.Lookup(name) + if obj != nil && obj.Type() == nil { + p.errorf("%v has nil type", obj) + } + // type alias if p.tok == '=' { - // type alias p.next() - typ := p.parseType(pkg) - if obj := scope.Lookup(name); obj != nil { - typ = obj.Type() // use previously imported type - if typ == nil { - p.errorf("%v (type alias) used in cycle", obj) - } - } else { - obj = types.NewTypeName(token.NoPos, pkg, name, typ) - scope.Insert(obj) + if obj != nil { + // use the previously imported (canonical) type + t := obj.Type() + p.update(t, nlist) + p.parseType(pkg) // discard + return t } - p.typeMap[n] = typ - return typ + t := p.parseType(pkg, nlist...) + obj = types.NewTypeName(token.NoPos, pkg, name, t) + scope.Insert(obj) + return t } - // named type - obj := scope.Lookup(name) + // defined type if obj == nil { - // a named type may be referred to before the underlying type - // is known - set it up + // A named type may be referred to before the underlying type + // is known - set it up. tname := types.NewTypeName(token.NoPos, pkg, name, nil) types.NewNamed(tname, nil, nil) scope.Insert(tname) obj = tname } - typ := obj.Type() - p.typeMap[n] = typ + // use the previously imported (canonical), or newly created type + t := obj.Type() + p.update(t, nlist) - nt, ok := typ.(*types.Named) + nt, ok := t.(*types.Named) if !ok { // This can happen for unsafe.Pointer, which is a TypeName holding a Basic type. pt := p.parseType(pkg) - if pt != typ { + if pt != t { p.error("unexpected underlying type for non-named TypeName") } - return typ + return t } underlying := p.parseType(pkg) @@ -449,41 +477,70 @@ func (p *parser) parseNamedType(n int) types.Type { return nt } -func (p *parser) parseInt() int64 { +func (p *parser) parseInt64() int64 { lit := p.expect(scanner.Int) - n, err := strconv.ParseInt(lit, 10, 0) + n, err := strconv.ParseInt(lit, 10, 64) if err != nil { p.error(err) } return n } +func (p *parser) parseInt() int { + lit := p.expect(scanner.Int) + n, err := strconv.ParseInt(lit, 10, 0 /* int */) + if err != nil { + p.error(err) + } + return int(n) +} + // ArrayOrSliceType = "[" [ int ] "]" Type . -func (p *parser) parseArrayOrSliceType(pkg *types.Package) types.Type { +func (p *parser) parseArrayOrSliceType(pkg *types.Package, nlist []int) types.Type { p.expect('[') if p.tok == ']' { p.next() - return types.NewSlice(p.parseType(pkg)) + + t := new(types.Slice) + p.update(t, nlist) + + *t = *types.NewSlice(p.parseType(pkg)) + return t } - n := p.parseInt() + t := new(types.Array) + p.update(t, nlist) + + len := p.parseInt64() p.expect(']') - return types.NewArray(p.parseType(pkg), n) + + *t = *types.NewArray(p.parseType(pkg), len) + return t } // MapType = "map" "[" Type "]" Type . -func (p *parser) parseMapType(pkg *types.Package) types.Type { +func (p *parser) parseMapType(pkg *types.Package, nlist []int) types.Type { p.expectKeyword("map") + + t := new(types.Map) + p.update(t, nlist) + p.expect('[') key := p.parseType(pkg) p.expect(']') elem := p.parseType(pkg) - return types.NewMap(key, elem) + + *t = *types.NewMap(key, elem) + return t } // ChanType = "chan" ["<-" | "-<"] Type . -func (p *parser) parseChanType(pkg *types.Package) types.Type { +func (p *parser) parseChanType(pkg *types.Package, nlist []int) types.Type { p.expectKeyword("chan") + + t := new(types.Chan) + p.update(t, nlist) + dir := types.SendRecv switch p.tok { case '-': @@ -500,13 +557,17 @@ func (p *parser) parseChanType(pkg *types.Package) types.Type { } } - return types.NewChan(dir, p.parseType(pkg)) + *t = *types.NewChan(dir, p.parseType(pkg)) + return t } // StructType = "struct" "{" { Field } "}" . -func (p *parser) parseStructType(pkg *types.Package) types.Type { +func (p *parser) parseStructType(pkg *types.Package, nlist []int) types.Type { p.expectKeyword("struct") + t := new(types.Struct) + p.update(t, nlist) + var fields []*types.Var var tags []string @@ -519,7 +580,8 @@ func (p *parser) parseStructType(pkg *types.Package) types.Type { } p.expect('}') - return types.NewStruct(fields, tags) + *t = *types.NewStruct(fields, tags) + return t } // ParamList = "(" [ { Parameter "," } Parameter ] ")" . @@ -562,10 +624,15 @@ func (p *parser) parseResultList(pkg *types.Package) *types.Tuple { } // FunctionType = ParamList ResultList . -func (p *parser) parseFunctionType(pkg *types.Package) *types.Signature { +func (p *parser) parseFunctionType(pkg *types.Package, nlist []int) *types.Signature { + t := new(types.Signature) + p.update(t, nlist) + params, isVariadic := p.parseParamList(pkg) results := p.parseResultList(pkg) - return types.NewSignature(nil, params, results, isVariadic) + + *t = *types.NewSignature(nil, params, results, isVariadic) + return t } // Func = Name FunctionType . @@ -577,13 +644,16 @@ func (p *parser) parseFunc(pkg *types.Package) *types.Func { p.discardDirectiveWhileParsingTypes(pkg) return nil } - return types.NewFunc(token.NoPos, pkg, name, p.parseFunctionType(pkg)) + return types.NewFunc(token.NoPos, pkg, name, p.parseFunctionType(pkg, nil)) } // InterfaceType = "interface" "{" { ("?" Type | Func) ";" } "}" . -func (p *parser) parseInterfaceType(pkg *types.Package) types.Type { +func (p *parser) parseInterfaceType(pkg *types.Package, nlist []int) types.Type { p.expectKeyword("interface") + t := new(types.Interface) + p.update(t, nlist) + var methods []*types.Func var embeddeds []types.Type @@ -600,53 +670,61 @@ func (p *parser) parseInterfaceType(pkg *types.Package) types.Type { } p.expect('}') - return types.NewInterfaceType(methods, embeddeds) + *t = *types.NewInterfaceType(methods, embeddeds) + return t } // PointerType = "*" ("any" | Type) . -func (p *parser) parsePointerType(pkg *types.Package) types.Type { +func (p *parser) parsePointerType(pkg *types.Package, nlist []int) types.Type { p.expect('*') if p.tok == scanner.Ident { p.expectKeyword("any") - return types.Typ[types.UnsafePointer] + t := types.Typ[types.UnsafePointer] + p.update(t, nlist) + return t } - return types.NewPointer(p.parseType(pkg)) + + t := new(types.Pointer) + p.update(t, nlist) + + *t = *types.NewPointer(p.parseType(pkg)) + + return t } -// TypeDefinition = NamedType | MapType | ChanType | StructType | InterfaceType | PointerType | ArrayOrSliceType | FunctionType . -func (p *parser) parseTypeDefinition(pkg *types.Package, n int) types.Type { - var t types.Type +// TypeSpec = NamedType | MapType | ChanType | StructType | InterfaceType | PointerType | ArrayOrSliceType | FunctionType . +func (p *parser) parseTypeSpec(pkg *types.Package, nlist []int) types.Type { switch p.tok { case scanner.String: - t = p.parseNamedType(n) + return p.parseNamedType(nlist) case scanner.Ident: switch p.lit { case "map": - t = p.parseMapType(pkg) + return p.parseMapType(pkg, nlist) case "chan": - t = p.parseChanType(pkg) + return p.parseChanType(pkg, nlist) case "struct": - t = p.parseStructType(pkg) + return p.parseStructType(pkg, nlist) case "interface": - t = p.parseInterfaceType(pkg) + return p.parseInterfaceType(pkg, nlist) } case '*': - t = p.parsePointerType(pkg) + return p.parsePointerType(pkg, nlist) case '[': - t = p.parseArrayOrSliceType(pkg) + return p.parseArrayOrSliceType(pkg, nlist) case '(': - t = p.parseFunctionType(pkg) + return p.parseFunctionType(pkg, nlist) } - p.typeMap[n] = t - return t + p.errorf("expected type name or literal, got %s", scanner.TokenString(p.tok)) + return nil } const ( @@ -700,29 +778,39 @@ func lookupBuiltinType(typ int) types.Type { }[typ] } -// Type = "<" "type" ( "-" int | int [ TypeDefinition ] ) ">" . -func (p *parser) parseType(pkg *types.Package) (t types.Type) { +// Type = "<" "type" ( "-" int | int [ TypeSpec ] ) ">" . +// +// parseType updates the type map to t for all type numbers n. +// +func (p *parser) parseType(pkg *types.Package, n ...int) (t types.Type) { p.expect('<') p.expectKeyword("type") switch p.tok { case scanner.Int: - n := p.parseInt() - + n1 := p.parseInt() if p.tok == '>' { - t = p.typeMap[int(n)] + t = p.typeMap[n1] + switch t { + case nil: + p.errorf("invalid type number, type %d not yet declared", n1) + case reserved: + p.errorf("invalid type cycle, type %d not yet defined", n1) + } + p.update(t, n) } else { - t = p.parseTypeDefinition(pkg, int(n)) + p.reserve(n1) + t = p.parseTypeSpec(pkg, append(n, n1)) } case '-': p.next() - n := p.parseInt() - t = lookupBuiltinType(int(n)) + n1 := p.parseInt() + t = lookupBuiltinType(n1) + p.update(t, n) default: p.errorf("expected type number, got %s (%q)", scanner.TokenString(p.tok), p.lit) - return nil } p.expect('>') @@ -735,7 +823,7 @@ func (p *parser) parsePackageInit() PackageInit { initfunc := p.parseUnquotedString() priority := -1 if p.version == "v1" { - priority = int(p.parseInt()) + priority = p.parseInt() } return PackageInit{Name: name, InitFunc: initfunc, Priority: priority} } @@ -781,7 +869,7 @@ func (p *parser) parseInitDataDirective() { case "priority": p.next() - p.initdata.Priority = int(p.parseInt()) + p.initdata.Priority = p.parseInt() p.expect(';') case "init": @@ -795,8 +883,8 @@ func (p *parser) parseInitDataDirective() { p.next() // The graph data is thrown away for now. for p.tok != ';' && p.tok != scanner.EOF { - p.parseInt() - p.parseInt() + p.parseInt64() + p.parseInt64() } p.expect(';') diff --git a/src/go/internal/gccgoimporter/testdata/aliases.go b/src/go/internal/gccgoimporter/testdata/aliases.go new file mode 100644 index 0000000000..cfb59b3e31 --- /dev/null +++ b/src/go/internal/gccgoimporter/testdata/aliases.go @@ -0,0 +1,65 @@ +package aliases + +type ( + T0 [10]int + T1 []byte + T2 struct { + x int + } + T3 interface { + m() T2 + } + T4 func(int, T0) chan T2 +) + +// basic aliases +type ( + Ai = int + A0 = T0 + A1 = T1 + A2 = T2 + A3 = T3 + A4 = T4 + + A10 = [10]int + A11 = []byte + A12 = struct { + x int + } + A13 = interface { + m() A2 + } + A14 = func(int, A0) chan A2 +) + +// alias receiver types +func (T0) m1() {} +func (A0) m2() {} + +// alias receiver types (long type declaration chains) +type ( + V0 = V1 + V1 = (V2) + V2 = (V3) + V3 = T0 +) + +func (V1) n() {} + +// cycles +type C0 struct { + f1 C1 + f2 C2 +} + +type ( + C1 *C0 + C2 = C1 +) + +type ( + C5 struct { + f *C6 + } + C6 = C5 +) diff --git a/src/go/internal/gccgoimporter/testdata/aliases.gox b/src/go/internal/gccgoimporter/testdata/aliases.gox new file mode 100644 index 0000000000..2428c06874 --- /dev/null +++ b/src/go/internal/gccgoimporter/testdata/aliases.gox @@ -0,0 +1,33 @@ +v2; +package aliases; +prefix go; +package aliases go.aliases go.aliases; +type > + func (? ) .go.aliases.m1 (); + func (? ) .go.aliases.m2 (); + func (? >>>) .go.aliases.n (); +>>; +type >>>; +type >>; +type >>; +type ; }>>; +type ; }>>>; }>>; +type , ? ) >>>; +type ; +type ; }>>>; +type , ? ) >>>>; +type >; +type >>; .go.aliases.f2 >; }>>; +type ; +type ; +type >>; }>>; +type ; +type ; +type ; +type ; +type ; +type ; +type >; +type ; +type ; +type ; diff --git a/src/go/internal/gccgoimporter/testdata/issue27856.go b/src/go/internal/gccgoimporter/testdata/issue27856.go new file mode 100644 index 0000000000..bf361e1cd8 --- /dev/null +++ b/src/go/internal/gccgoimporter/testdata/issue27856.go @@ -0,0 +1,9 @@ +package lib + +type M struct { + E E +} +type F struct { + _ *M +} +type E = F diff --git a/src/go/internal/gccgoimporter/testdata/issue27856.gox b/src/go/internal/gccgoimporter/testdata/issue27856.gox new file mode 100644 index 0000000000..6665e64021 --- /dev/null +++ b/src/go/internal/gccgoimporter/testdata/issue27856.gox @@ -0,0 +1,9 @@ +v2; +package main; +pkgpath main; +import runtime runtime "runtime"; +init runtime runtime..import sys runtime_internal_sys..import; +init_graph 0 1; +type ; }>>>; }>>>; +type ; +type ;