]> Cypherpunks.ru repositories - gostls13.git/commitdiff
cmd/compile: switch to final unified IR export format
authorMatthew Dempsky <mdempsky@google.com>
Tue, 15 Feb 2022 19:39:41 +0000 (11:39 -0800)
committerMatthew Dempsky <mdempsky@google.com>
Mon, 4 Apr 2022 18:33:29 +0000 (18:33 +0000)
Now that there's a native go/types importer for unified IR, the
compiler no longer needs to stay backwards compatible with old iexport
importers.

This CL also updates the go/types and go/internal/gcimporter tests to
expect that the unified IR importer sets the receiver parameter type
to the underlying Interface type, rather than the Named type. This is
a temporary workaround until we make a decision on #49906.

Notably, this makes `GOEXPERIMENT=unified go test` work on generics
code without requiring `-vet=off` (because previously cmd/vet was
relying on unified IR's backwards-compatible iexport data, which
omitted generic types).

Change-Id: Iac7a2346bb7a91e6690fb2978fb702fadae5559d
Reviewed-on: https://go-review.googlesource.com/c/go/+/386004
Trust: Matthew Dempsky <mdempsky@google.com>
Run-TryBot: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>

src/cmd/compile/internal/importer/gcimporter_test.go
src/cmd/compile/internal/noder/export.go
src/cmd/compile/internal/noder/import.go
src/cmd/compile/internal/noder/reader.go
src/cmd/compile/internal/noder/unified.go
src/go/internal/gcimporter/gcimporter_test.go
src/go/types/eval_test.go

index 9fecf742fb96ce19c0d103fcff4e015f2fc95595..3b6d77747bc15edc20ce8f803b9d68ff2ddf0d64 100644 (file)
@@ -115,10 +115,14 @@ func TestImportTestdata(t *testing.T) {
        }
 
        testfiles := map[string][]string{
-               "exports.go": {"go/ast", "go/token"},
+               "exports.go":  {"go/ast", "go/token"},
+               "generics.go": nil,
        }
-       if !goexperiment.Unified {
-               testfiles["generics.go"] = nil
+       if goexperiment.Unified {
+               // TODO(mdempsky): Fix test below to flatten the transitive
+               // Package.Imports graph. Unified IR is more precise about
+               // recreating the package import graph.
+               testfiles["exports.go"] = []string{"go/ast"}
        }
 
        for testfile, wantImports := range testfiles {
@@ -326,6 +330,14 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types2.Named, level int) {
                return // not an interface
        }
 
+       // The unified IR importer always sets interface method receiver
+       // parameters to point to the Interface type, rather than the Named.
+       // See #49906.
+       var want types2.Type = named
+       if goexperiment.Unified {
+               want = iface
+       }
+
        // check explicitly declared methods
        for i := 0; i < iface.NumExplicitMethods(); i++ {
                m := iface.ExplicitMethod(i)
@@ -334,7 +346,7 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types2.Named, level int) {
                        t.Errorf("%s: missing receiver type", m)
                        continue
                }
-               if recv.Type() != named {
+               if recv.Type() != want {
                        t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), named)
                }
        }
index 1a296e22c8489b803fedf0ed2965daa1fe8be5c1..263cdc262bb88d0ec1662b7f229b8f6027f38686 100644 (file)
@@ -14,52 +14,22 @@ import (
        "cmd/internal/bio"
 )
 
-// writeNewExportFunc is a hook that can be added to append extra
-// export data after the normal export data section. It allows
-// experimenting with new export data format designs without requiring
-// immediate support in the go/internal or x/tools importers.
-var writeNewExportFunc func(out io.Writer)
-
 func WriteExports(out *bio.Writer) {
-       // When unified IR exports are enable, we simply append it to the
-       // end of the normal export data (with compiler extensions
-       // disabled), and write an extra header giving its size.
-       //
-       // If the compiler sees this header, it knows to read the new data
-       // instead; meanwhile the go/types importers will silently ignore it
-       // and continue processing the old export instead.
-       //
-       // This allows us to experiment with changes to the new export data
-       // format without needing to update the go/internal/gcimporter or
-       // (worse) x/tools/go/gcexportdata.
-
-       useNewExport := writeNewExportFunc != nil
-
-       var old, new bytes.Buffer
-
-       typecheck.WriteExports(&old, !useNewExport)
-
-       if useNewExport {
-               writeNewExportFunc(&new)
-       }
-
-       oldLen := old.Len()
-       newLen := new.Len()
+       var data bytes.Buffer
 
-       if useNewExport {
-               fmt.Fprintf(out, "\nnewexportsize %v\n", newLen)
+       if base.Debug.Unified != 0 {
+               data.WriteByte('u')
+               writeUnifiedExport(&data)
+       } else {
+               typecheck.WriteExports(&data, true)
        }
 
        // The linker also looks for the $$ marker - use char after $$ to distinguish format.
        out.WriteString("\n$$B\n") // indicate binary export format
-       io.Copy(out, &old)
+       io.Copy(out, &data)
        out.WriteString("\n$$\n")
-       io.Copy(out, &new)
 
        if base.Debug.Export != 0 {
-               fmt.Printf("BenchmarkExportSize:%s 1 %d bytes\n", base.Ctxt.Pkgpath, oldLen)
-               if useNewExport {
-                       fmt.Printf("BenchmarkNewExportSize:%s 1 %d bytes\n", base.Ctxt.Pkgpath, newLen)
-               }
+               fmt.Printf("BenchmarkExportSize:%s 1 %d bytes\n", base.Ctxt.Pkgpath, data.Len())
        }
 }
index 7ba1b23d12eb9d228945627bffb7fb645a17c487..2cef9f75e80e9db0e9c408b241ebca8abd57ecdb 100644 (file)
@@ -8,10 +8,10 @@ import (
        "errors"
        "fmt"
        "internal/buildcfg"
+       "internal/pkgbits"
        "os"
        pathpkg "path"
        "runtime"
-       "strconv"
        "strings"
        "unicode"
        "unicode/utf8"
@@ -28,22 +28,6 @@ import (
        "cmd/internal/objabi"
 )
 
-// haveLegacyImports records whether we've imported any packages
-// without a new export data section. This is useful for experimenting
-// with new export data format designs, when you need to support
-// existing tests that manually compile files with inconsistent
-// compiler flags.
-var haveLegacyImports = false
-
-// newReadImportFunc is an extension hook for experimenting with new
-// export data formats. If a new export data payload was written out
-// for an imported package by overloading writeNewExportFunc, then
-// that payload will be mapped into memory and passed to
-// newReadImportFunc.
-var newReadImportFunc = func(data string, pkg1 *types.Pkg, env *types2.Context, packages map[string]*types2.Package) (pkg2 *types2.Package, err error) {
-       panic("unexpected new export data payload")
-}
-
 type gcimports struct {
        ctxt     *types2.Context
        packages map[string]*types2.Package
@@ -220,7 +204,7 @@ func readImportFile(path string, target *ir.Package, env *types2.Context, packag
        }
        defer f.Close()
 
-       r, end, newsize, err := findExportData(f)
+       r, end, err := findExportData(f)
        if err != nil {
                return
        }
@@ -229,41 +213,40 @@ func readImportFile(path string, target *ir.Package, env *types2.Context, packag
                fmt.Printf("importing %s (%s)\n", path, f.Name())
        }
 
-       if newsize != 0 {
-               // We have unified IR data. Map it, and feed to the importers.
-               end -= newsize
-               var data string
-               data, err = base.MapFile(r.File(), end, newsize)
-               if err != nil {
-                       return
-               }
+       c, err := r.ReadByte()
+       if err != nil {
+               return
+       }
 
-               pkg2, err = newReadImportFunc(data, pkg1, env, packages)
-       } else {
-               // We only have old data. Oh well, fall back to the legacy importers.
-               haveLegacyImports = true
+       pos := r.Offset()
 
-               var c byte
-               switch c, err = r.ReadByte(); {
-               case err != nil:
-                       return
+       // Map export data section into memory as a single large
+       // string. This reduces heap fragmentation and allows returning
+       // individual substrings very efficiently.
+       var data string
+       data, err = base.MapFile(r.File(), pos, end-pos)
+       if err != nil {
+               return
+       }
 
-               case c != 'i':
-                       // Indexed format is distinguished by an 'i' byte,
-                       // whereas previous export formats started with 'c', 'd', or 'v'.
-                       err = fmt.Errorf("unexpected package format byte: %v", c)
-                       return
+       switch c {
+       case 'u':
+               if !buildcfg.Experiment.Unified {
+                       base.Fatalf("unexpected export data format")
                }
 
-               pos := r.Offset()
+               // TODO(mdempsky): This seems a bit clunky.
+               data = strings.TrimSuffix(data, "\n$$\n")
 
-               // Map string (and data) section into memory as a single large
-               // string. This reduces heap fragmentation and allows
-               // returning individual substrings very efficiently.
-               var data string
-               data, err = base.MapFile(r.File(), pos, end-pos)
-               if err != nil {
-                       return
+               pr := pkgbits.NewPkgDecoder(pkg1.Path, data)
+
+               // Read package descriptors for both types2 and compiler backend.
+               readPackage(newPkgReader(pr), pkg1)
+               pkg2 = importer.ReadPackage(env, packages, pr)
+
+       case 'i':
+               if buildcfg.Experiment.Unified {
+                       base.Fatalf("unexpected export data format")
                }
 
                typecheck.ReadImports(pkg1, data)
@@ -274,6 +257,12 @@ func readImportFile(path string, target *ir.Package, env *types2.Context, packag
                                return
                        }
                }
+
+       default:
+               // Indexed format is distinguished by an 'i' byte,
+               // whereas previous export formats started with 'c', 'd', or 'v'.
+               err = fmt.Errorf("unexpected package format byte: %v", c)
+               return
        }
 
        err = addFingerprint(path, f, end)
@@ -283,7 +272,7 @@ func readImportFile(path string, target *ir.Package, env *types2.Context, packag
 // findExportData returns a *bio.Reader positioned at the start of the
 // binary export data section, and a file offset for where to stop
 // reading.
-func findExportData(f *os.File) (r *bio.Reader, end, newsize int64, err error) {
+func findExportData(f *os.File) (r *bio.Reader, end int64, err error) {
        r = bio.NewReader(f)
 
        // check object header
@@ -326,14 +315,6 @@ func findExportData(f *os.File) (r *bio.Reader, end, newsize int64, err error) {
 
        // process header lines
        for !strings.HasPrefix(line, "$$") {
-               if strings.HasPrefix(line, "newexportsize ") {
-                       fields := strings.Fields(line)
-                       newsize, err = strconv.ParseInt(fields[1], 10, 64)
-                       if err != nil {
-                               return
-                       }
-               }
-
                line, err = r.ReadString('\n')
                if err != nil {
                        return
index 71efac80aa7223a7f12dd342855a3873943be102..1350c22467f98ac9314e07816d0b86ab9ffda8f6 100644 (file)
@@ -589,10 +589,6 @@ func (pr *pkgReader) objIdx(idx int, implicits, explicits []*types.Type) ir.Node
                if pri, ok := objReader[sym]; ok {
                        return pri.pr.objIdx(pri.idx, nil, explicits)
                }
-               if haveLegacyImports {
-                       assert(len(explicits) == 0)
-                       return typecheck.Resolve(ir.NewIdent(src.NoXPos, sym))
-               }
                base.Fatalf("unresolved stub: %v", sym)
        }
 
@@ -1972,12 +1968,6 @@ func InlineCall(call *ir.CallExpr, fn *ir.Func, inlIndex int) *ir.InlinedCallExp
 
        pri, ok := bodyReader[fn]
        if !ok {
-               // Assume it's an imported function or something that we don't
-               // have access to in quirks mode.
-               if haveLegacyImports {
-                       return nil
-               }
-
                base.FatalfAt(call.Pos(), "missing function body for call to %v", fn)
        }
 
index f45c4a7ea857f4cc9b48f74001d0da8bc5e39ce0..2c1f2362ad9bcc08e93383b846f242562d58d7b1 100644 (file)
@@ -16,7 +16,6 @@ import (
        "sort"
 
        "cmd/compile/internal/base"
-       "cmd/compile/internal/importer"
        "cmd/compile/internal/inline"
        "cmd/compile/internal/ir"
        "cmd/compile/internal/typecheck"
@@ -74,17 +73,6 @@ var localPkgReader *pkgReader
 func unified(noders []*noder) {
        inline.NewInline = InlineCall
 
-       writeNewExportFunc = writeNewExport
-
-       newReadImportFunc = func(data string, pkg1 *types.Pkg, ctxt *types2.Context, packages map[string]*types2.Package) (pkg2 *types2.Package, err error) {
-               pr := pkgbits.NewPkgDecoder(pkg1.Path, data)
-
-               // Read package descriptors for both types2 and compiler backend.
-               readPackage(newPkgReader(pr), pkg1)
-               pkg2 = importer.ReadPackage(ctxt, packages, pr)
-               return
-       }
-
        data := writePkgStub(noders)
 
        // We already passed base.Flag.Lang to types2 to handle validating
@@ -266,7 +254,7 @@ func readPackage(pr *pkgReader, importpkg *types.Pkg) {
        }
 }
 
-func writeNewExport(out io.Writer) {
+func writeUnifiedExport(out io.Writer) {
        l := linker{
                pw: pkgbits.NewPkgEncoder(base.Debug.SyncFrames),
 
@@ -332,5 +320,5 @@ func writeNewExport(out io.Writer) {
                w.Flush()
        }
 
-       l.pw.DumpTo(out)
+       base.Ctxt.Fingerprint = l.pw.DumpTo(out)
 }
index 89b7fde83661ca249a391781bc933bcd682af331..c10915fdf5124cfd12de16353819b7d150d6ee39 100644 (file)
@@ -125,10 +125,14 @@ func TestImportTestdata(t *testing.T) {
        }
 
        testfiles := map[string][]string{
-               "exports.go": {"go/ast", "go/token"},
+               "exports.go":  {"go/ast", "go/token"},
+               "generics.go": nil,
        }
-       if !goexperiment.Unified {
-               testfiles["generics.go"] = nil
+       if goexperiment.Unified {
+               // TODO(mdempsky): Fix test below to flatten the transitive
+               // Package.Imports graph. Unified IR is more precise about
+               // recreating the package import graph.
+               testfiles["exports.go"] = []string{"go/ast"}
        }
 
        for testfile, wantImports := range testfiles {
@@ -153,11 +157,6 @@ func TestImportTestdata(t *testing.T) {
 }
 
 func TestImportTypeparamTests(t *testing.T) {
-       // This test doesn't yet work with the unified export format.
-       if goexperiment.Unified {
-               t.Skip("unified export data format is currently unsupported")
-       }
-
        // This package only handles gc export data.
        if runtime.Compiler != "gc" {
                t.Skipf("gc-built packages not available (compiler = %s)", runtime.Compiler)
@@ -460,6 +459,14 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) {
                return // not an interface
        }
 
+       // The unified IR importer always sets interface method receiver
+       // parameters to point to the Interface type, rather than the Named.
+       // See #49906.
+       var want types.Type = named
+       if goexperiment.Unified {
+               want = iface
+       }
+
        // check explicitly declared methods
        for i := 0; i < iface.NumExplicitMethods(); i++ {
                m := iface.ExplicitMethod(i)
@@ -468,8 +475,8 @@ func verifyInterfaceMethodRecvs(t *testing.T, named *types.Named, level int) {
                        t.Errorf("%s: missing receiver type", m)
                        continue
                }
-               if recv.Type() != named {
-                       t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), named)
+               if recv.Type() != want {
+                       t.Errorf("%s: got recv type %s; want %s", m, recv.Type(), want)
                }
        }
 
index b0745c16d9729b84b5e26853d7623ed94892ba7a..6f5b548eb2c6e1db4fad34774364350121070978 100644 (file)
@@ -12,6 +12,7 @@ import (
        "go/importer"
        "go/parser"
        "go/token"
+       "internal/goexperiment"
        "internal/testenv"
        "strings"
        "testing"
@@ -208,7 +209,7 @@ func TestCheckExpr(t *testing.T) {
        // expr is an identifier or selector expression that is passed
        // to CheckExpr at the position of the comment, and object is
        // the string form of the object it denotes.
-       const src = `
+       src := `
 package p
 
 import "fmt"
@@ -235,6 +236,13 @@ func f(a int, s string) S {
        return S{}
 }`
 
+       // The unified IR importer always sets interface method receiver
+       // parameters to point to the Interface type, rather than the Named.
+       // See #49906.
+       if goexperiment.Unified {
+               src = strings.ReplaceAll(src, "func (fmt.Stringer).", "func (interface).")
+       }
+
        fset := token.NewFileSet()
        f, err := parser.ParseFile(fset, "p", src, parser.ParseComments)
        if err != nil {