Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

go/types: interfaces must be marked complete before they can be compared #34151

Closed
neild opened this issue Sep 6, 2019 · 7 comments
Closed
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages)
Milestone

Comments

@neild
Copy link
Contributor

neild commented Sep 6, 2019

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

$ go version
go version devel +2da9c3e0f9 Fri Sep 6 18:03:49 2019 +0000 linux/amd64

Does this issue reproduce with the latest release?

With tip.

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

go env Output
$ go env
$ go env
GO111MODULE=""
GOARCH="amd64"
GOBIN=""
GOCACHE="/usr/local/google/home/dneil/.cache/go-build"
GOENV="/usr/local/google/home/dneil/.config/go/env"
GOEXE=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/usr/local/google/home/dneil/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/lib/google-golang"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/lib/google-golang/pkg/tool/linux_amd64"
GCCGO="/usr/bin/gccgo"
AR="ar"
CC="clang"
CXX="clang++"
CGO_ENABLED="1"
GOMOD="/usr/local/google/home/dneil/src/bug/go.mod"
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 -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -fdebug-prefix-map=/tmp/go-build907626480=/tmp/go-build -gno-record-gcc-switches"

What did you do?

// go.mod
module play.ground

go 1.13
// main.go
package main

import (
        "play.ground/p"
)

type T struct {
        F interface {
                p.I
        }
}

var _ = p.T(T{})

func main() {}
// p/p.go
package p

type I interface{ M() }
type T struct {
        F interface {
                I
        }
}
$ go vet ./main.go
# command-line-arguments
vet: ./main.go:13:13: cannot convert (T literal) (value of type T) to p.T

What did you expect to see?

No error, since T and p.T have identical underlying types.

What did you see instead?

A vet error.

@FiloSottile FiloSottile added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 10, 2019
@FiloSottile FiloSottile added this to the Go1.14 milestone Sep 10, 2019
@FiloSottile
Copy link
Contributor

This bisects to a80c5f0 by @griesemer.

commit a80c5f0588e3835ff37c45a15d179571d9e403e0
Author: Robert Griesemer <gri@golang.org>
Date:   Wed Aug 21 17:13:45 2019 -0700

    go/types: allow embedding overlapping interfaces

This is breaking thousands of tests inside Google, but I can't revert it because it's part of the language change which supposedly is already in use, so marking P1 (Soon).

/cc @mdempsky @golang/osp-team

@FiloSottile FiloSottile added the Soon This needs to be done soon. (regressions, serious bugs, outages) label Sep 16, 2019
@griesemer
Copy link
Contributor

on it

@mdempsky
Copy link
Member

Hm, tangentially, I suppose cmd/compile should complain about overlapping interfaces when -lang=go1.13 or earlier is specified?

@FiloSottile
Copy link
Contributor

@griesemer To clarify, it's not causing visible internal breakage (we don't make people run tip), but it's blocking us from testing tip against Google tests as we do every week, because they all break around protobufs. Not an emergency.

@neild
Copy link
Contributor Author

neild commented Sep 16, 2019

If it's just protobufs that are breaking, I think it's a one-line fix to get it working for now. Just delete this line:
https://go.googlesource.com/protobuf/+/refs/heads/master/proto/decode.go#39

@dmitshur
Copy link
Contributor

Or move that line to a _test.go file, to make it a protobuf test failure rather than a protobuf build failure.

@gopherbot
Copy link

Change https://golang.org/cl/195837 mentions this issue: go/types: make sure interfaces are complete before comparing them

@griesemer griesemer changed the title cmd/vet: regression in literal conversion go/types: interfaces must be marked complete before they can be compared Sep 16, 2019
@golang golang locked and limited conversation to collaborators Sep 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker Soon This needs to be done soon. (regressions, serious bugs, outages)
Projects
None yet
Development

No branches or pull requests

6 participants