go/types: type-check embedded methods in wrong scope #25008

nabice · 2018-04-23T10:06:28Z

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (`go version`)?

go version devel +d11df8baa8 Sun Apr 22 22:32:11 2018 +0000 linux/amd64

Does this issue reproduce with the latest release?

no, only recent commit d11df8b

What operating system and processor architecture are you using (`go env`)?

GOARCH="amd64"
GOBIN=""
GOCACHE="/home/nabice/.cache/go-build"
GOEXE=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"
GOPATH="/home/nabice/go:/home/nabice/skygo/go"
GORACE=""
GOROOT="/home/nabice/source/go"
GOTMPDIR=""
GOTOOLDIR="/home/nabice/source/go/pkg/tool/linux_amd64"
GCCGO="gccgo"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
CGO_CFLAGS="-g -O2"
CGO_CPPFLAGS=""
CGO_CXXFLAGS="-g -O2"
CGO_FFLAGS="-g -O2"
CGO_LDFLAGS="-g -O2"
PKG_CONFIG="pkg-config"
GOGCCFLAGS="-fPIC -m64 -pthread -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build162693936=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Two go files:
a.go

package main

import (
        "fmt"
        "io"
)

type A interface {
        io.Reader
}

type A1 struct {
}

func (d *A1) Read(p []byte) (n int, err error) {
        fmt.Println("A1")
        return 0, nil
}

func bbb(t A) {
        t.Read(nil)
}

func main() {
        a := new(A1)
        bbb(a)
}

b.go

package main

type B interface {
    A
}

go vet a.go b.go runs OK
go vet b.go a.go failed.

The reason is that, in file src/go/types/interfaces.go:226, We pass wrong context(scope) to lookup for "io". So typecheck cannot found io.Reader.

stack trace:

 0  0x000000000062ab05 in go/types.(*Checker).infoFromTypeLit
    at ./interfaces.go:226
 1  0x000000000062b45a in go/types.(*Checker).infoFromTypeName
    at ./interfaces.go:339
 2  0x000000000062a91a in go/types.(*Checker).infoFromTypeLit
    at ./interfaces.go:224
 3  0x0000000000658272 in go/types.(*Checker).interfaceType
    at ./typexpr.go:545

When we check b.go firstly, go tries to get A's info from b.go's scope

What did you expect to see?

No error found

What did you see instead?

# command-line-arguments

./a.go:21:4: invalid operation: t (variable of type A) has no field or method Read
vet: typecheck failures

The text was updated successfully, but these errors were encountered:

FiloSottile · 2018-04-24T17:26:47Z

/cc @griesemer

Looks similar to #23914 and #24140

griesemer · 2018-04-24T20:16:28Z

@nabice Thanks for this report and the (correct) analysis! Fix forthcoming.

A shorter reproducer:

a.go

package p

import "io"

type A interface {
        io.Reader
}

func f(a A) {
        a.Read(nil)
}

b.go

package p

type B interface {
    A
}

gopherbot · 2018-04-24T22:07:06Z

Change https://golang.org/cl/109139 mentions this issue: go/types: use correct (file) scopes when computing interface method sets

myitcv · 2018-04-27T08:06:15Z

@griesemer we just had another instance of this via gopherjs/gopherjs#808 - would you consider back-porting this fix to the 1.10 series?

myitcv · 2018-04-27T08:10:06Z

Edit: fixed typo below in second paragraph.

Just to add a bit more colour.

Using 1.10.1, type checking gorgonia.org/gorgonia with go/types succeeds when using importer.For("gc", nil), but fails with importer.For("source", nil).

Using tip it succeeds in both cases (the fix was introduced by https://go-review.googlesource.com/c/go/+/109139).

Here are the repro steps I used:

cd `mktemp -d`
export GOPATH=$PWD
go get gorgonia.org/gorgonia
go install $(go list -f '{{join .Deps " "}}' gorgonia.org/gorgonia )
cat <<EOD > check.go
// +build ignore

package main

import (
	"fmt"
	"go/ast"
	"go/build"
	"go/importer"
	"go/parser"
	"go/token"
	"go/types"
	"os"
	"path/filepath"
)

func main() {
	bpkg, err := build.Import("gorgonia.org/gorgonia", ".", 0)
	if err != nil {
		fatalf("failed to import .: %v", err)
	}

	fset := token.NewFileSet()
	var files []*ast.File

	for _, f := range bpkg.GoFiles {
		fp := filepath.Join(bpkg.Dir, f)
		fi, err := os.Open(fp)
		if err != nil {
			fatalf("failed to open %v: %v", fp, err)
		}

		pf, err := parser.ParseFile(fset, fp, fi, 0)
		if err != nil {
			fatalf("failed to parse %v: %v", fp, err)
		}

		files = append(files, pf)
	}

	{
		conf := types.Config{
			Importer: importer.For("gc", nil),
		}

		_, err = conf.Check("gorgonia.org/gorgonia", fset, files, nil)
		if err != nil {
			fatalf("type checking failed using gc importer: %v", err)
		}
	}

	{
		conf := types.Config{
			Importer: importer.For("source", nil),
		}

		_, err = conf.Check("gorgonia.org/gorgonia", fset, files, nil)
		if err != nil {
			fatalf("type checking failed using source importer: %v", err)
		}
	}
}

func fatalf(format string, args ...interface{}) {
	fmt.Fprintf(os.Stderr, format+"\n", args...)
	os.Exit(1)
}
EOD
go run check.go

griesemer · 2018-04-30T22:57:05Z

@myitcv While the CL https://go-review.googlesource.com/c/go/+/109139 fixed this specific issue, the fix is in a (significant) chunk of code that doesn't even exist in the 1.10 version of go/types (https://tip.golang.org/src/go/types/interfaces.go was added, and https://golang.org/src/go/types/ordering.go was removed). So back-porting this fix implies a much larger change than what meets the eye.

You may be able to (temporarily) work around the problem by re-arranging some your source code (relative order of method and interface declarations). It doesn't always work but it's worth a try.

Is there a reason not to move to 1.11 in gorgonia, or perhaps (for now) vendor the latest version of go/types?

myitcv · 2018-05-01T06:36:39Z

the fix is in a (significant) chunk of code that doesn't even exist in the 1.10 version of go/types

Ah, I hadn't realised this. Thank you for clarifying.

... or perhaps (for now) vendor the latest version of go/types?

Excellent suggestion, thank you. We can do that within GopherJS (which is ultimately where we get bitten by this) until we release GopherJS 1.11 that corresponds to Go 1.11.

Per golang/go#25008 (comment) the fix for method set calculation will not be back ported to Go 1.10. Instead gri suggests we vendor the latest version of go/types. This PR does just that. It is effectively a temporary sticking plaster until Go 1.11 lands, or more specifically until we release GopherJS for Go 1.11. In order to successfully vendor go/types, however, we need to move compiler/vendor to the repo root (because build also uses go/types). And because golang.org/x/tools/go/gcexportdata and golang.org/x/tools/go/types/typeutil also reference go/types we need to vendor them as well. vendor/vendor.json is updated using govendor, with the one exception being a manual addition for go/types (govendor doesn't appear to understand how to vendor a standard library package). Fixes gopherjs#808.

FiloSottile changed the title ~~type-check embedded methods in wrong scope~~ go/types: type-check embedded methods in wrong scope Apr 24, 2018

FiloSottile added the NeedsFix label Apr 24, 2018

FiloSottile added this to the Go1.11 milestone Apr 24, 2018

FiloSottile added the release-blocker label Apr 24, 2018

griesemer self-assigned this Apr 24, 2018

gopherbot closed this as completed in 73becbf Apr 25, 2018

myitcv mentioned this issue Apr 27, 2018

Interface "missing methods" gopherjs/gopherjs#808

Closed

myitcv mentioned this issue May 1, 2018

vendor: vendor go/types to workaround bug in method set calc. gopherjs/gopherjs#811

Closed

golang locked and limited conversation to collaborators May 1, 2019

gopherbot added the FrozenDueToAge label May 1, 2019

rsc unassigned griesemer Jun 23, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

go/types: type-check embedded methods in wrong scope #25008

go/types: type-check embedded methods in wrong scope #25008

nabice commented Apr 23, 2018 •

edited by FiloSottile

Loading

FiloSottile commented Apr 24, 2018

griesemer commented Apr 24, 2018

gopherbot commented Apr 24, 2018

myitcv commented Apr 27, 2018

myitcv commented Apr 27, 2018 •

edited

Loading

griesemer commented Apr 30, 2018

myitcv commented May 1, 2018

go/types: type-check embedded methods in wrong scope #25008

go/types: type-check embedded methods in wrong scope #25008

Comments

nabice commented Apr 23, 2018 • edited by FiloSottile Loading

What version of Go are you using (go version)?

Does this issue reproduce with the latest release?

What operating system and processor architecture are you using (go env)?

What did you do?

Two go files: a.go

b.go

What did you expect to see?

What did you see instead?

FiloSottile commented Apr 24, 2018

griesemer commented Apr 24, 2018

gopherbot commented Apr 24, 2018

myitcv commented Apr 27, 2018

myitcv commented Apr 27, 2018 • edited Loading

griesemer commented Apr 30, 2018

myitcv commented May 1, 2018

nabice commented Apr 23, 2018 •

edited by FiloSottile

Loading

What version of Go are you using (`go version`)?

What operating system and processor architecture are you using (`go env`)?

Two go files:
a.go

myitcv commented Apr 27, 2018 •

edited

Loading