Skip to content

Commit e51e352

Browse files
committed
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 <[email protected]> TryBot-Result: Gobot Gobot <[email protected]> Reviewed-by: Robert Griesemer <[email protected]>
1 parent 34bb05f commit e51e352

File tree

7 files changed

+382
-99
lines changed

7 files changed

+382
-99
lines changed

go/internal/gccgoimporter/gccgoinstallation_test.go

Lines changed: 15 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,14 @@ package gccgoimporter
99

1010
import (
1111
"go/types"
12-
"runtime"
1312
"testing"
1413
)
1514

15+
// importablePackages is a list of packages that we verify that we can
16+
// import. This should be all standard library packages in all relevant
17+
// versions of gccgo. Note that since gccgo follows a different release
18+
// cycle, and since different systems have different versions installed,
19+
// we can't use the last-two-versions rule of the gc toolchain.
1620
var importablePackages = [...]string{
1721
"archive/tar",
1822
"archive/zip",
@@ -59,7 +63,7 @@ var importablePackages = [...]string{
5963
"encoding/binary",
6064
"encoding/csv",
6165
"encoding/gob",
62-
"encoding",
66+
// "encoding", // Added in GCC 4.9.
6367
"encoding/hex",
6468
"encoding/json",
6569
"encoding/pem",
@@ -71,7 +75,7 @@ var importablePackages = [...]string{
7175
"go/ast",
7276
"go/build",
7377
"go/doc",
74-
"go/format",
78+
// "go/format", // Added in GCC 4.8.
7579
"go/parser",
7680
"go/printer",
7781
"go/scanner",
@@ -84,7 +88,7 @@ var importablePackages = [...]string{
8488
"html",
8589
"html/template",
8690
"image/color",
87-
"image/color/palette",
91+
// "image/color/palette", // Added in GCC 4.9.
8892
"image/draw",
8993
"image/gif",
9094
"image",
@@ -103,7 +107,7 @@ var importablePackages = [...]string{
103107
"mime/multipart",
104108
"net",
105109
"net/http/cgi",
106-
"net/http/cookiejar",
110+
// "net/http/cookiejar", // Added in GCC 4.8.
107111
"net/http/fcgi",
108112
"net/http",
109113
"net/http/httptest",
@@ -147,14 +151,14 @@ var importablePackages = [...]string{
147151
}
148152

149153
func TestInstallationImporter(t *testing.T) {
150-
// This test relies on gccgo being around, which it most likely will be if we
151-
// were compiled with gccgo.
152-
if runtime.Compiler != "gccgo" {
154+
// This test relies on gccgo being around.
155+
gpath := gccgoPath()
156+
if gpath == "" {
153157
t.Skip("This test needs gccgo")
154158
}
155159

156160
var inst GccgoInstallation
157-
err := inst.InitFromDriver("gccgo")
161+
err := inst.InitFromDriver(gpath)
158162
if err != nil {
159163
t.Fatal(err)
160164
}
@@ -179,12 +183,12 @@ func TestInstallationImporter(t *testing.T) {
179183

180184
// Test for certain specific entities in the imported data.
181185
for _, test := range [...]importerTest{
182-
{pkgpath: "io", name: "Reader", want: "type Reader interface{Read(p []uint8) (n int, err error)}"},
186+
{pkgpath: "io", name: "Reader", want: "type Reader interface{Read(p []byte) (n int, err error)}"},
183187
{pkgpath: "io", name: "ReadWriter", want: "type ReadWriter interface{Reader; Writer}"},
184188
{pkgpath: "math", name: "Pi", want: "const Pi untyped float"},
185189
{pkgpath: "math", name: "Sin", want: "func Sin(x float64) float64"},
186190
{pkgpath: "sort", name: "Ints", want: "func Ints(a []int)"},
187-
{pkgpath: "unsafe", name: "Pointer", want: "type Pointer unsafe.Pointer"},
191+
{pkgpath: "unsafe", name: "Pointer", want: "type Pointer"},
188192
} {
189193
runImporterTest(t, imp, nil, &test)
190194
}

go/internal/gccgoimporter/importer.go

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -65,6 +65,7 @@ func findExportFile(searchpaths []string, pkgpath string) (string, error) {
6565
const (
6666
gccgov1Magic = "v1;\n"
6767
gccgov2Magic = "v2;\n"
68+
gccgov3Magic = "v3;\n"
6869
goimporterMagic = "\n$$ "
6970
archiveMagic = "!<ar"
7071
)
@@ -93,7 +94,7 @@ func openExportFile(fpath string) (reader io.ReadSeeker, closer io.Closer, err e
9394

9495
var elfreader io.ReaderAt
9596
switch string(magic[:]) {
96-
case gccgov1Magic, gccgov2Magic, goimporterMagic:
97+
case gccgov1Magic, gccgov2Magic, gccgov3Magic, goimporterMagic:
9798
// Raw export data.
9899
reader = f
99100
return
@@ -198,7 +199,7 @@ func GetImporter(searchpaths []string, initmap map[*types.Package]InitData) Impo
198199
}
199200

200201
switch magics {
201-
case gccgov1Magic, gccgov2Magic:
202+
case gccgov1Magic, gccgov2Magic, gccgov3Magic:
202203
var p parser
203204
p.init(fpath, reader, imports)
204205
pkg = p.parsePackage()

go/internal/gccgoimporter/importer_test.go

Lines changed: 58 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ import (
1414
"os"
1515
"os/exec"
1616
"path/filepath"
17-
"runtime"
17+
"regexp"
18+
"strconv"
1819
"testing"
1920
)
2021

@@ -56,9 +57,6 @@ func runImporterTest(t *testing.T, imp Importer, initmap map[*types.Package]Init
5657
// Check that the package's own init function has the package's priority
5758
for _, pkginit := range initdata.Inits {
5859
if pkginit.InitFunc == test.wantinits[0] {
59-
if initdata.Priority != pkginit.Priority {
60-
t.Errorf("%s: got self priority %d; want %d", test.pkgpath, pkginit.Priority, initdata.Priority)
61-
}
6260
found = true
6361
break
6462
}
@@ -68,27 +66,11 @@ func runImporterTest(t *testing.T, imp Importer, initmap map[*types.Package]Init
6866
t.Errorf("%s: could not find expected function %q", test.pkgpath, test.wantinits[0])
6967
}
7068

71-
// Each init function in the list other than the first one is a
72-
// dependency of the function immediately before it. Check that
73-
// the init functions appear in descending priority order.
74-
priority := initdata.Priority
75-
for _, wantdepinit := range test.wantinits[1:] {
76-
found = false
77-
for _, pkginit := range initdata.Inits {
78-
if pkginit.InitFunc == wantdepinit {
79-
if priority <= pkginit.Priority {
80-
t.Errorf("%s: got dep priority %d; want less than %d", test.pkgpath, pkginit.Priority, priority)
81-
}
82-
found = true
83-
priority = pkginit.Priority
84-
break
85-
}
86-
}
87-
88-
if !found {
89-
t.Errorf("%s: could not find expected function %q", test.pkgpath, wantdepinit)
90-
}
91-
}
69+
// FIXME: the original version of this test was written against
70+
// the v1 export data scheme for capturing init functions, so it
71+
// verified the priority values. We moved away from the priority
72+
// scheme some time ago; it is not clear how much work it would be
73+
// to validate the new init export data.
9274
}
9375
}
9476

@@ -103,17 +85,17 @@ var importerTests = [...]importerTest{
10385
{pkgpath: "time", name: "Nanosecond", want: "const Nanosecond Duration", wantval: "1"},
10486
{pkgpath: "unicode", name: "IsUpper", want: "func IsUpper(r rune) bool"},
10587
{pkgpath: "unicode", name: "MaxRune", want: "const MaxRune untyped rune", wantval: "1114111"},
106-
{pkgpath: "imports", wantinits: []string{"imports..import", "fmt..import", "math..import"}},
88+
{pkgpath: "imports", wantinits: []string{"imports..import", "fmt..import"}},
10789
{pkgpath: "importsar", name: "Hello", want: "var Hello string"},
10890
{pkgpath: "aliases", name: "A14", want: "type A14 = func(int, T0) chan T2"},
10991
{pkgpath: "aliases", name: "C0", want: "type C0 struct{f1 C1; f2 C1}"},
11092
{pkgpath: "escapeinfo", name: "NewT", want: "func NewT(data []byte) *T"},
11193
{pkgpath: "issue27856", name: "M", want: "type M struct{E F}"},
94+
{pkgpath: "v1reflect", name: "Type", want: "type Type interface{Align() int; AssignableTo(u Type) bool; Bits() int; ChanDir() ChanDir; Elem() Type; Field(i int) StructField; FieldAlign() int; FieldByIndex(index []int) StructField; FieldByName(name string) (StructField, bool); FieldByNameFunc(match func(string) bool) (StructField, bool); Implements(u Type) bool; In(i int) Type; IsVariadic() bool; Key() Type; Kind() Kind; Len() int; Method(int) Method; MethodByName(string) (Method, bool); Name() string; NumField() int; NumIn() int; NumMethod() int; NumOut() int; Out(i int) Type; PkgPath() string; Size() uintptr; String() string; common() *commonType; rawString() string; runtimeType() *runtimeType; uncommon() *uncommonType}"},
11295
}
11396

11497
func TestGoxImporter(t *testing.T) {
115-
testenv.MustHaveGoBuild(t)
116-
98+
testenv.MustHaveExec(t) // this is to skip nacl, js
11799
initmap := make(map[*types.Package]InitData)
118100
imp := GetImporter([]string{"testdata"}, initmap)
119101

@@ -122,15 +104,46 @@ func TestGoxImporter(t *testing.T) {
122104
}
123105
}
124106

125-
func TestObjImporter(t *testing.T) {
126-
testenv.MustHaveGoBuild(t)
107+
// gccgoPath returns a path to gccgo if it is present (either in
108+
// path or specified via GCCGO environment variable), or an
109+
// empty string if no gccgo is available.
110+
func gccgoPath() string {
111+
gccgoname := os.Getenv("GCCGO")
112+
if gccgoname == "" {
113+
gccgoname = "gccgo"
114+
}
115+
if gpath, gerr := exec.LookPath(gccgoname); gerr == nil {
116+
return gpath
117+
}
118+
return ""
119+
}
127120

128-
// This test relies on gccgo being around, which it most likely will be if we
129-
// were compiled with gccgo.
130-
if runtime.Compiler != "gccgo" {
121+
func TestObjImporter(t *testing.T) {
122+
// This test relies on gccgo being around.
123+
gpath := gccgoPath()
124+
if gpath == "" {
131125
t.Skip("This test needs gccgo")
132126
}
133127

128+
verout, err := exec.Command(gpath, "--version").CombinedOutput()
129+
if err != nil {
130+
t.Logf("%s", verout)
131+
t.Fatal(err)
132+
}
133+
vers := regexp.MustCompile(`([0-9]+)\.([0-9]+)`).FindSubmatch(verout)
134+
if len(vers) == 0 {
135+
t.Fatalf("could not find version number in %s", verout)
136+
}
137+
major, err := strconv.Atoi(string(vers[1]))
138+
if err != nil {
139+
t.Fatal(err)
140+
}
141+
minor, err := strconv.Atoi(string(vers[2]))
142+
if err != nil {
143+
t.Fatal(err)
144+
}
145+
t.Logf("gccgo version %d.%d", major, minor)
146+
134147
tmpdir, err := ioutil.TempDir("", "")
135148
if err != nil {
136149
t.Fatal(err)
@@ -146,11 +159,22 @@ func TestObjImporter(t *testing.T) {
146159
arimp := GetImporter([]string{artmpdir}, arinitmap)
147160

148161
for _, test := range importerTests {
162+
// Support for type aliases was added in GCC 7.
163+
if test.pkgpath == "aliases" || test.pkgpath == "issue27856" {
164+
if major < 7 {
165+
t.Logf("skipping %q: not supported before gccgo version 7", test.pkgpath)
166+
continue
167+
}
168+
}
169+
149170
gofile := filepath.Join("testdata", test.pkgpath+".go")
171+
if _, err := os.Stat(gofile); os.IsNotExist(err) {
172+
continue
173+
}
150174
ofile := filepath.Join(tmpdir, test.pkgpath+".o")
151175
afile := filepath.Join(artmpdir, "lib"+test.pkgpath+".a")
152176

153-
cmd := exec.Command("gccgo", "-fgo-pkgpath="+test.pkgpath, "-c", "-o", ofile, gofile)
177+
cmd := exec.Command(gpath, "-fgo-pkgpath="+test.pkgpath, "-c", "-o", ofile, gofile)
154178
out, err := cmd.CombinedOutput()
155179
if err != nil {
156180
t.Logf("%s", out)

0 commit comments

Comments
 (0)