From e51e3523bf428a351d970ea09324e3600f0998f7 Mon Sep 17 00:00:00 2001 From: Than McIntosh Date: Thu, 29 Nov 2018 11:37:58 -0500 Subject: [PATCH] go/internal/gccgoimporter: update package to match std lib version This CL updates the importer to match the original code in the std lib but for the necessary changes to make the code work in x/tools and with older versions of the std lib. Notably, it brings over changes from: https://go-review.googlesource.com/c/152078 https://go-review.googlesource.com/c/152077 https://golang.org/cl/151997 https://golang.org/cl/151557 https://golang.org/cl/149957 including test fixes (we want tests to run when gccgo is available, not just when all go tools are gccgo-based), bug fixes (primarily related to aliases), performance enhancements, and new code to read the V3 export data emitted by the most recent gccgo. Change-Id: I2d34bace23769e62795599b93db8295169076594 Reviewed-on: https://go-review.googlesource.com/c/151717 Run-TryBot: Than McIntosh TryBot-Result: Gobot Gobot Reviewed-by: Robert Griesemer --- .../gccgoimporter/gccgoinstallation_test.go | 26 +- go/internal/gccgoimporter/importer.go | 5 +- go/internal/gccgoimporter/importer_test.go | 92 +++-- go/internal/gccgoimporter/parser.go | 330 +++++++++++++++--- go/internal/gccgoimporter/parser_test.go | 3 +- .../gccgoimporter/testdata/v1reflect.gox | Bin 0 -> 10872 bytes go/internal/gccgoimporter/testenv_test.go | 25 ++ 7 files changed, 382 insertions(+), 99 deletions(-) create mode 100644 go/internal/gccgoimporter/testdata/v1reflect.gox diff --git a/go/internal/gccgoimporter/gccgoinstallation_test.go b/go/internal/gccgoimporter/gccgoinstallation_test.go index 4cd7f67058..f5bc22b36f 100644 --- a/go/internal/gccgoimporter/gccgoinstallation_test.go +++ b/go/internal/gccgoimporter/gccgoinstallation_test.go @@ -9,10 +9,14 @@ package gccgoimporter import ( "go/types" - "runtime" "testing" ) +// importablePackages is a list of packages that we verify that we can +// import. This should be all standard library packages in all relevant +// versions of gccgo. Note that since gccgo follows a different release +// cycle, and since different systems have different versions installed, +// we can't use the last-two-versions rule of the gc toolchain. var importablePackages = [...]string{ "archive/tar", "archive/zip", @@ -59,7 +63,7 @@ var importablePackages = [...]string{ "encoding/binary", "encoding/csv", "encoding/gob", - "encoding", + // "encoding", // Added in GCC 4.9. "encoding/hex", "encoding/json", "encoding/pem", @@ -71,7 +75,7 @@ var importablePackages = [...]string{ "go/ast", "go/build", "go/doc", - "go/format", + // "go/format", // Added in GCC 4.8. "go/parser", "go/printer", "go/scanner", @@ -84,7 +88,7 @@ var importablePackages = [...]string{ "html", "html/template", "image/color", - "image/color/palette", + // "image/color/palette", // Added in GCC 4.9. "image/draw", "image/gif", "image", @@ -103,7 +107,7 @@ var importablePackages = [...]string{ "mime/multipart", "net", "net/http/cgi", - "net/http/cookiejar", + // "net/http/cookiejar", // Added in GCC 4.8. "net/http/fcgi", "net/http", "net/http/httptest", @@ -147,14 +151,14 @@ var importablePackages = [...]string{ } func TestInstallationImporter(t *testing.T) { - // This test relies on gccgo being around, which it most likely will be if we - // were compiled with gccgo. - if runtime.Compiler != "gccgo" { + // This test relies on gccgo being around. + gpath := gccgoPath() + if gpath == "" { t.Skip("This test needs gccgo") } var inst GccgoInstallation - err := inst.InitFromDriver("gccgo") + err := inst.InitFromDriver(gpath) if err != nil { t.Fatal(err) } @@ -179,12 +183,12 @@ func TestInstallationImporter(t *testing.T) { // Test for certain specific entities in the imported data. for _, test := range [...]importerTest{ - {pkgpath: "io", name: "Reader", want: "type Reader interface{Read(p []uint8) (n int, err error)}"}, + {pkgpath: "io", name: "Reader", want: "type Reader interface{Read(p []byte) (n int, err error)}"}, {pkgpath: "io", name: "ReadWriter", want: "type ReadWriter interface{Reader; Writer}"}, {pkgpath: "math", name: "Pi", want: "const Pi untyped float"}, {pkgpath: "math", name: "Sin", want: "func Sin(x float64) float64"}, {pkgpath: "sort", name: "Ints", want: "func Ints(a []int)"}, - {pkgpath: "unsafe", name: "Pointer", want: "type Pointer unsafe.Pointer"}, + {pkgpath: "unsafe", name: "Pointer", want: "type Pointer"}, } { runImporterTest(t, imp, nil, &test) } diff --git a/go/internal/gccgoimporter/importer.go b/go/internal/gccgoimporter/importer.go index a6366df9ac..1094af2c56 100644 --- a/go/internal/gccgoimporter/importer.go +++ b/go/internal/gccgoimporter/importer.go @@ -65,6 +65,7 @@ func findExportFile(searchpaths []string, pkgpath string) (string, error) { const ( gccgov1Magic = "v1;\n" gccgov2Magic = "v2;\n" + gccgov3Magic = "v3;\n" goimporterMagic = "\n$$ " archiveMagic = "! package object typeList []types.Type // type number -> type + typeData []string // unparsed type data (v3 and later) + fixups []fixupRecord // fixups to apply at end of parsing initdata InitData // package init priority data } +// When reading V1 export data it's possible to encounter a defined +// type N1 with an underlying defined type N2 while we are still +// reading in that defined type N2; see issue #29006 for an instance +// of this. Example: +// +// type N1 N2 +// type N2 struct { +// ... +// p *N1 +// } +// +// To handle such cases, the parser generates a fixup record (below) and +// delays setting of N1's underlying type until parsing is complete, at +// which point fixups are applied. + +type fixupRecord struct { + toUpdate *types.Named // type to modify when fixup is processed + target types.Type // type that was incomplete when fixup was created +} + func (p *parser) init(filename string, src io.Reader, imports map[string]*types.Package) { - p.scanner.Init(src) - p.scanner.Error = func(_ *scanner.Scanner, msg string) { p.error(msg) } - p.scanner.Mode = scanner.ScanIdents | scanner.ScanInts | scanner.ScanFloats | scanner.ScanStrings | scanner.ScanComments | scanner.SkipComments - p.scanner.Whitespace = 1<<'\t' | 1<<'\n' | 1<<' ' - p.scanner.Filename = filename // for good error messages - p.next() + p.scanner = new(scanner.Scanner) + p.initScanner(filename, src) p.imports = imports p.typeList = make([]types.Type, 1 /* type numbers start at 1 */, 16) } +func (p *parser) initScanner(filename string, src io.Reader) { + p.scanner.Init(src) + p.scanner.Error = func(_ *scanner.Scanner, msg string) { p.error(msg) } + p.scanner.Mode = scanner.ScanIdents | scanner.ScanInts | scanner.ScanFloats | scanner.ScanStrings + p.scanner.Whitespace = 1<<'\t' | 1<<' ' + p.scanner.Filename = filename // for good error messages + p.next() +} + type importError struct { pos scanner.Position err error @@ -75,6 +103,13 @@ func (p *parser) expect(tok rune) string { return lit } +func (p *parser) expectEOL() { + if p.version == "v1" || p.version == "v2" { + p.expect(';') + } + p.expect('\n') +} + func (p *parser) expectKeyword(keyword string) { lit := p.expect(scanner.Ident) if lit != keyword { @@ -100,7 +135,7 @@ func (p *parser) parseUnquotedString() string { buf.WriteString(p.scanner.TokenText()) // This loop needs to examine each character before deciding whether to consume it. If we see a semicolon, // we need to let it be consumed by p.next(). - for ch := p.scanner.Peek(); ch != ';' && ch != scanner.EOF && p.scanner.Whitespace&(1<' { + if len(p.typeData) > 0 && p.typeList[n1] == nil { + p.parseSavedType(pkg, n1, n) + } t = p.typeList[n1] - if t == reserved { - p.errorf("invalid type cycle, type %d not yet defined", n1) + if len(p.typeData) == 0 && t == reserved { + p.errorf("invalid type cycle, type %d not yet defined (nlist=%v)", n1, n) } p.update(t, n) } else { @@ -812,12 +907,124 @@ func (p *parser) parseType(pkg *types.Package, n ...int) (t types.Type) { default: p.errorf("expected type number, got %s (%q)", scanner.TokenString(p.tok), p.lit) + return nil + } + + if t == nil || t == reserved { + p.errorf("internal error: bad return from parseType(%v)", n) } p.expect('>') return } +// InlineBody = "" .{NN} +// Reports whether a body was skipped. +func (p *parser) skipInlineBody() { + // We may or may not have seen the '<' already, depending on + // whether the function had a result type or not. + if p.tok == '<' { + p.next() + p.expectKeyword("inl") + } else if p.tok != scanner.Ident || p.lit != "inl" { + return + } else { + p.next() + } + + p.expect(':') + want := p.parseInt() + p.expect('>') + + defer func(w uint64) { + p.scanner.Whitespace = w + }(p.scanner.Whitespace) + p.scanner.Whitespace = 0 + + got := 0 + for got < want { + r := p.scanner.Next() + if r == scanner.EOF { + p.error("unexpected EOF") + } + got += utf8.RuneLen(r) + } +} + +// Types = "types" maxp1 exportedp1 (offset length)* . +func (p *parser) parseTypes(pkg *types.Package) { + maxp1 := p.parseInt() + exportedp1 := p.parseInt() + p.typeList = make([]types.Type, maxp1, maxp1) + + type typeOffset struct { + offset int + length int + } + var typeOffsets []typeOffset + + total := 0 + for i := 1; i < maxp1; i++ { + len := p.parseInt() + typeOffsets = append(typeOffsets, typeOffset{total, len}) + total += len + } + + defer func(w uint64) { + p.scanner.Whitespace = w + }(p.scanner.Whitespace) + p.scanner.Whitespace = 0 + + // We should now have p.tok pointing to the final newline. + // The next runes from the scanner should be the type data. + + var sb strings.Builder + for sb.Len() < total { + r := p.scanner.Next() + if r == scanner.EOF { + p.error("unexpected EOF") + } + sb.WriteRune(r) + } + allTypeData := sb.String() + + p.typeData = []string{""} // type 0, unused + for _, to := range typeOffsets { + p.typeData = append(p.typeData, allTypeData[to.offset:to.offset+to.length]) + } + + for i := 1; i < int(exportedp1); i++ { + p.parseSavedType(pkg, i, []int{}) + } +} + +// parseSavedType parses one saved type definition. +func (p *parser) parseSavedType(pkg *types.Package, i int, nlist []int) { + defer func(s *scanner.Scanner, tok rune, lit string) { + p.scanner = s + p.tok = tok + p.lit = lit + }(p.scanner, p.tok, p.lit) + + p.scanner = new(scanner.Scanner) + p.initScanner(p.scanner.Filename, strings.NewReader(p.typeData[i])) + p.expectKeyword("type") + id := p.parseInt() + if id != i { + p.errorf("type ID mismatch: got %d, want %d", id, i) + } + if p.typeList[i] == reserved { + p.errorf("internal error: %d already reserved in parseSavedType", i) + } + if p.typeList[i] == nil { + p.reserve(i) + p.parseTypeSpec(pkg, append(nlist, i)) + } + if p.typeList[i] == nil || p.typeList[i] == reserved { + p.errorf("internal error: parseSavedType(%d,%v) reserved/nil", i, nlist) + } +} + // PackageInit = unquotedString unquotedString int . func (p *parser) parsePackageInit() PackageInit { name := p.parseUnquotedString() @@ -833,7 +1040,7 @@ func (p *parser) parsePackageInit() PackageInit { func (p *parser) discardDirectiveWhileParsingTypes(pkg *types.Package) { for { switch p.tok { - case ';': + case '\n', ';': return case '<': p.parseType(pkg) @@ -852,7 +1059,7 @@ func (p *parser) maybeCreatePackage() { } } -// InitDataDirective = ( "v1" | "v2" ) ";" | +// InitDataDirective = ( "v1" | "v2" | "v3" ) ";" | // "priority" int ";" | // "init" { PackageInit } ";" | // "checksum" unquotedString ";" . @@ -863,31 +1070,32 @@ func (p *parser) parseInitDataDirective() { } switch p.lit { - case "v1", "v2": + case "v1", "v2", "v3": p.version = p.lit p.next() p.expect(';') + p.expect('\n') case "priority": p.next() p.initdata.Priority = p.parseInt() - p.expect(';') + p.expectEOL() case "init": p.next() - for p.tok != ';' && p.tok != scanner.EOF { + for p.tok != '\n' && p.tok != ';' && p.tok != scanner.EOF { p.initdata.Inits = append(p.initdata.Inits, p.parsePackageInit()) } - p.expect(';') + p.expectEOL() case "init_graph": p.next() // The graph data is thrown away for now. - for p.tok != ';' && p.tok != scanner.EOF { + for p.tok != '\n' && p.tok != ';' && p.tok != scanner.EOF { p.parseInt64() p.parseInt64() } - p.expect(';') + p.expectEOL() case "checksum": // Don't let the scanner try to parse the checksum as a number. @@ -897,7 +1105,7 @@ func (p *parser) parseInitDataDirective() { p.scanner.Mode &^= scanner.ScanInts | scanner.ScanFloats p.next() p.parseUnquotedString() - p.expect(';') + p.expectEOL() default: p.errorf("unexpected identifier: %q", p.lit) @@ -909,6 +1117,7 @@ func (p *parser) parseInitDataDirective() { // "pkgpath" unquotedString ";" | // "prefix" unquotedString ";" | // "import" unquotedString unquotedString string ";" | +// "indirectimport" unquotedString unquotedstring ";" | // "func" Func ";" | // "type" Type ";" | // "var" Var ";" | @@ -920,29 +1129,29 @@ func (p *parser) parseDirective() { } switch p.lit { - case "v1", "v2", "priority", "init", "init_graph", "checksum": + case "v1", "v2", "v3", "priority", "init", "init_graph", "checksum": p.parseInitDataDirective() case "package": p.next() p.pkgname = p.parseUnquotedString() p.maybeCreatePackage() - if p.version == "v2" && p.tok != ';' { + if p.version != "v1" && p.tok != '\n' && p.tok != ';' { p.parseUnquotedString() p.parseUnquotedString() } - p.expect(';') + p.expectEOL() case "pkgpath": p.next() p.pkgpath = p.parseUnquotedString() p.maybeCreatePackage() - p.expect(';') + p.expectEOL() case "prefix": p.next() p.pkgpath = p.parseUnquotedString() - p.expect(';') + p.expectEOL() case "import": p.next() @@ -950,7 +1159,19 @@ func (p *parser) parseDirective() { pkgpath := p.parseUnquotedString() p.getPkg(pkgpath, pkgname) p.parseString() - p.expect(';') + p.expectEOL() + + case "indirectimport": + p.next() + pkgname := p.parseUnquotedString() + pkgpath := p.parseUnquotedString() + p.getPkg(pkgpath, pkgname) + p.expectEOL() + + case "types": + p.next() + p.parseTypes(p.pkg) + p.expectEOL() case "func": p.next() @@ -958,24 +1179,24 @@ func (p *parser) parseDirective() { if fun != nil { p.pkg.Scope().Insert(fun) } - p.expect(';') + p.expectEOL() case "type": p.next() p.parseType(p.pkg) - p.expect(';') + p.expectEOL() case "var": p.next() v := p.parseVar(p.pkg) p.pkg.Scope().Insert(v) - p.expect(';') + p.expectEOL() case "const": p.next() c := p.parseConst(p.pkg) p.pkg.Scope().Insert(c) - p.expect(';') + p.expectEOL() default: p.errorf("unexpected identifier: %q", p.lit) @@ -987,6 +1208,13 @@ func (p *parser) parsePackage() *types.Package { for p.tok != scanner.EOF { p.parseDirective() } + for _, f := range p.fixups { + if f.target.Underlying() == nil { + p.errorf("internal error: fixup can't be applied, loop required") + } + f.toUpdate.SetUnderlying(f.target.Underlying()) + } + p.fixups = nil for _, typ := range p.typeList { if it, ok := typ.(*types.Interface); ok { it.Complete() diff --git a/go/internal/gccgoimporter/parser_test.go b/go/internal/gccgoimporter/parser_test.go index c3fcce1e00..32deac219b 100644 --- a/go/internal/gccgoimporter/parser_test.go +++ b/go/internal/gccgoimporter/parser_test.go @@ -22,7 +22,7 @@ var typeParserTests = []struct { {id: "foo", typ: ">", want: "*error"}, {id: "foo", typ: "", want: "unsafe.Pointer"}, {id: "foo", typ: ">>", want: "foo.Bar", underlying: "*foo.Bar"}, - {id: "foo", typ: " func (? ) M (); >", want: "bar.Foo", underlying: "int8", methods: "func (bar.Foo).M()"}, + {id: "foo", typ: "\nfunc (? ) M ();\n>", want: "bar.Foo", underlying: "int8", methods: "func (bar.Foo).M()"}, {id: "foo", typ: ">", want: "bar.foo", underlying: "int8"}, {id: "foo", typ: ">", want: "[]int8"}, {id: "foo", typ: ">", want: "[42]int8"}, @@ -39,6 +39,7 @@ func TestTypeParser(t *testing.T) { for _, test := range typeParserTests { var p parser p.init("test.gox", strings.NewReader(test.typ), make(map[string]*types.Package)) + p.version = "v2" p.pkgname = test.id p.pkgpath = test.id p.maybeCreatePackage() diff --git a/go/internal/gccgoimporter/testdata/v1reflect.gox b/go/internal/gccgoimporter/testdata/v1reflect.gox new file mode 100644 index 0000000000000000000000000000000000000000..ea468414d9fa8ad66e978799a8ff5d2f90eb4961 GIT binary patch literal 10872 zcmb^%U2o&KF?-iOG*9hwUkIZpoI?^sl4Zxp)kVIx4es^>P3|6A6br2*Cqit=mE>&P z!=itnKcx@-iJc)i=IOMCfv1OMQa`eRGAqigUW@2#KU*w|vK9+Okbipk zw`Tz5DzCEzP}9$z{$Vkyx||mIv!G{B|6myFd@3sZ)5E_Ou*$1smJ0posV9Q-tR`{k zd3>xzjjXU{EHLtE@fd{U(oX{k&-Q04km-E6O!Iruuf(_YdYOtq^nOg{tF))zLtNQv zT8p2=vDWD^AZlnNzERT#qG!O^SnVJUpXH>_lYJqBtzYZqhbC}>a56ct zWXVpVe_&82XYJ`Q=e;(9!9*+hQQx9bcffli?Irq|=6w_dz^K0nRvTe5OJ~z5(hwAt7gqiI#k6iCLuP4dFWQfGaE%r<6vd-$&c|J zCQdUeJQY10A&VS=Ly@~Yk)}^!jb38=!0~3WoTrO4uPf1C(G1iI*$va_c~wDHPCm}l zyJ7>{MTS)DR8-UDW0GHGr5z5(qNfL~)Pa$jszNy4D0TH?QfA40HnoJ00u=QcR%6FP zRH}5FrdzA!MJJ*Ms(}h8kw~B@uu1649UEBQC6uZoNGys2j1hf|RWeh}P+`e`N^8-7 zO4mmJ+!nAJwAs7atO7K$5W3S%exJVRktnj67JU`K=XsH@7sZN_vCrqlxmz)j(nd2J}pW|^=hHrr8SPkTZ8XbU@mu54rRtVm_yYHBu^XRs;QJI z(NKtnRNX?T3@jh0`fXW7BSM7A#PHe`B!c!t@yB8#ixLXk4 zh{m63wa@}J(r(*JY;6HADJWy1cLH~3VavL_EAAdM^577YOS`m2rab_?u*zB&uA5m9ax-H(k}vtF(3n^rGoB=c-nv z1kCTzObfvnQf{w-U+SRGiRbRlyB7IrHSyyU0 zXXy8hLO0F4e!=4}nAhJ8-zrJYaO&@~UJ-UX#@yMbqGX$uLBw|b5=Q;-2ueNKg;P4e zzh^rK<|iEKqA2FpH)Ln)qjj;aQ?@%9@a8r==@8&D$z2&b$phvsw(d6X!*0YU$7M1H zaWgt#8pJknLaR>G2mG|8Q@Hv3cAmk0nUNd$$(=gGUcrpnrc%O%It@_ah)MGhHt6KH zV}~3G9kU{U+YbNomt095Hocq|6>gdRBnsTKzHl-uR=8`prLcq7hC;Y$hPMpHfmMv$ zdXZ{2Q|c&!ra|{9Qi|QRiHgIG1#dIqd677d%(?-tT-q^epTOmAS?x{E>B6y$Z(R^b zfR~o?AZU9yIk@KNDvq4fq|YoPx=YZEu97;j{uD!Q2n8Kh;!&($)SK#UwoB^4{%xMI zOxT%4y4!9pc6Xn^vEC$0Z6bS|u+>B`33?TlCu281c}?bT(>FTVwrzm35SfYS?5@=+ zZsW75SK@5mJ%iKDi%*x7Ug$jKD8Lgm#irA8H2bxR9y|AB_qj4Y%*?^=6Mx7svdqjK z#MM_A*5}d7y~h1ZI(i3xj$~rO zGm$q6c$)zC=XLr*yD`;h-dFSo5sE!#ILra*}%o4()44=3b>82Q~PtPC1j@*@5Eq4A*y zu~zQKDa!9Y*bUoO0+`Aqfw$?WZJGjX-w4lsVZDuSc;C{y9zV?FTPIc0DS0PGoYt ziNyAR-eH;I(wJN(6#OW{h{wNa(aXoc;}WRrQ~30yAp-k)Wvd7-OZ=S4Sv1BoV<}I+ z`FjWgvv;X|=@562q2JByl_TVW%!Kw&uu?KNGng!@oV~3d2x_Du%y7z-2yEmb_zH+a z8<_|`1>#UQ7h!#|bm4f_(FlAyWWchzQ-{Un4l&g+wV+G0*I}$bCA%)m!h9+<9NLR@ zy=x?2!JB%y{WVPZ{T_WbGBo9XD6fMhb-s&HSL7;SVb{G%CC2|Vk1(n~KbJ@%j_)+P z8S)*Ih5wqCm=1W3=Qz{Hbox}S7UJq^Gzw2fko>f|Fl>ry=*>DNQ?0nU}-e3(AL^gi9MGQB9y;BVoLf5(a6Z-)9? zPDCWwZ{YXWIH3Q{0O$Ag0#Cxj=T~g}-+(9J{TF&1pTCCxzk>gq-hTj(XaLCk{vXU8 BTEYMT literal 0 HcmV?d00001 diff --git a/go/internal/gccgoimporter/testenv_test.go b/go/internal/gccgoimporter/testenv_test.go index 0db980f9fb..7afa464d9b 100644 --- a/go/internal/gccgoimporter/testenv_test.go +++ b/go/internal/gccgoimporter/testenv_test.go @@ -26,6 +26,20 @@ func HasGoBuild() bool { return true } +// HasExec reports whether the current system can start new processes +// using os.StartProcess or (more commonly) exec.Command. +func HasExec() bool { + switch runtime.GOOS { + case "nacl", "js": + return false + case "darwin": + if strings.HasPrefix(runtime.GOARCH, "arm") { + return false + } + } + return true +} + // MustHaveGoBuild checks that the current system can build programs with ``go build'' // and then run them with os.StartProcess or exec.Command. // If not, MustHaveGoBuild calls t.Skip with an explanation. @@ -35,10 +49,21 @@ func MustHaveGoBuild(t *testing.T) { } } +// MustHaveExec checks that the current system can start new processes +// using os.StartProcess or (more commonly) exec.Command. +// If not, MustHaveExec calls t.Skip with an explanation. +func MustHaveExec(t *testing.T) { + if !HasExec() { + t.Skipf("skipping test: cannot exec subprocess on %s/%s", runtime.GOOS, runtime.GOARCH) + } +} + var testenv = struct { HasGoBuild func() bool MustHaveGoBuild func(*testing.T) + MustHaveExec func(*testing.T) }{ HasGoBuild: HasGoBuild, MustHaveGoBuild: MustHaveGoBuild, + MustHaveExec: MustHaveExec, }