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

cmd/compile: import issue with self-referential generic interface type #48280

Closed
rogpeppe opened this issue Sep 9, 2021 · 7 comments
Closed

cmd/compile: import issue with self-referential generic interface type #48280

rogpeppe opened this issue Sep 9, 2021 · 7 comments

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Sep 9, 2021

go version devel go1.18-b86e8dd0f3 Thu Sep 9 09:06:46 2021 +0000 linux/amd64

I see an unexpected error when I run go test on the following code:

-- p.go --
package p

type I[T I[T]] interface {
	F() T
}
-- p_test.go --
package p

import "testing"

func TestP(t *testing.T) {}

The error that I see is:

# github.com/rogpeppe/generic/set-bug
vet: ./p.go:3:10: I is not a generic type
# github.com/rogpeppe/generic/set-bug.test
/tmp/go-build1456973644/b001/_testmain.go:13:8: could not import github.com/rogpeppe/generic/set-bug (cannot import "github.com/rogpeppe/generic/set-bug" (type parameter bound more than once), possibly version skew - reinstall package)
FAIL	github.com/rogpeppe/generic/set-bug [build failed]
@jayconrod
Copy link
Contributor

@jayconrod jayconrod commented Sep 9, 2021

This appears to be a compiler bug. I was able to reproduce it using this build script, outside of the go command:

#!/bin/bash

set -eu

work=$(mktemp -d)
echo '$work = '$work >&2
src=$(pwd)
goroot=$(go env GOROOT)

mkdir "$work/b001"
cat >"$work/b001/importcfg" <<EOF
packagefile testing=$goroot/pkg/linux_amd64/testing.a
EOF
go tool compile -o "$work/b001/out.a" -p example.com/test -lang=go1.18 -complete -importcfg="$work/b001/importcfg" -pack "$src/p.go" "$src/p_test.go"

mkdir "$work/b002"
cat >"$work/b002/importcfg" <<EOF
packagefile os=$goroot/pkg/linux_amd64/os.a
packagefile testing=$goroot/pkg/linux_amd64/testing.a
packagefile testing/internal/testdeps=$goroot/pkg/linux_amd64/testing/internal/testdeps.a
packagefile example.com/test=$work/b001/out.a
EOF
go tool compile -o "$work/b002/out.a" -p main -lang=go1.18 -complete -importcfg="$work/b002/importcfg" -pack "$src/_testmain.go"

_testmain.go is generated:


// Code generated by 'go test'. DO NOT EDIT.

package main

import (
        "os"

        "testing"
        "testing/internal/testdeps"


        _test "example.com/test"



)

var tests = []testing.InternalTest{

        {"TestP", _test.TestP},

}

var benchmarks = []testing.InternalBenchmark{

}

var examples = []testing.InternalExample{

}

func init() {
        testdeps.ImportPath = "example.com/test"
}



func main() {

        m := testing.MainStart(testdeps.TestDeps{}, tests, benchmarks, examples)

        os.Exit(m.Run())

}

Loading

@jayconrod jayconrod changed the title cmd/go: import issue with self-referential generic interface type cmd/compile: import issue with self-referential generic interface type Sep 9, 2021
@jayconrod jayconrod added this to the Go1.18 milestone Sep 9, 2021
@cuonglm
Copy link
Member

@cuonglm cuonglm commented Sep 9, 2021

This seems to be the problem in the importer. After creating stub declaration, it recursively declaring the type, which in turn named.SetTypeParams(tparams) called twice for the same tparams.

Maybe we can just check if the name already exists and returns before setup stub declaration:

if r.currPkg.Scope().Lookup(name) != nil {
	return
}

cc @findleyr

Loading

@findleyr
Copy link
Contributor

@findleyr findleyr commented Sep 9, 2021

Thanks for the report. Somewhat related: #48098. We need more controls around recursive instantiation.

CC @griesemer

Loading

@cuonglm
Copy link
Member

@cuonglm cuonglm commented Sep 9, 2021

Thanks for the report. Somewhat related: #48098. We need more controls around recursive instantiation.

CC @griesemer

Note that the package build ok with cmd/compile, and typecheck ok with go/types.

The problem only happens when import package.

Loading

@findleyr
Copy link
Contributor

@findleyr findleyr commented Sep 9, 2021

The problem only happens when import package.

Ack, right. Perhaps your suggested fix is the way to proceed. I will look into it.

Loading

@gopherbot
Copy link

@gopherbot gopherbot commented Sep 10, 2021

Change https://golang.org/cl/349009 mentions this issue: cmd/compile: prevent duplicated stub declaration in importReader.obj

Loading

@findleyr findleyr removed their assignment Sep 10, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Sep 10, 2021

Change https://golang.org/cl/349010 mentions this issue: go/internal/gcimporter: prevent duplicated stub declaration in importReader.obj

Loading

@gopherbot gopherbot closed this in b8c802b Sep 14, 2021
gopherbot pushed a commit that referenced this issue Sep 14, 2021
…wice

This is port of CL 349009 to go/internal/gcimporter.

Updates #48280

Change-Id: I7d40d8b67333538ca58fe012535d54e891d0ed16
Reviewed-on: https://go-review.googlesource.com/c/go/+/349010
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants