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
Labels
FrozenDueToAge generics Issue is related to generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Milestone

Comments

@rogpeppe
Copy link
Contributor

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]
@rogpeppe rogpeppe added the generics Issue is related to generics label Sep 9, 2021
@jayconrod
Copy link
Contributor

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())

}

@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
@jayconrod jayconrod added release-blocker NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Sep 9, 2021
@cuonglm
Copy link
Member

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

@findleyr
Copy link
Contributor

findleyr commented Sep 9, 2021

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

CC @griesemer

@cuonglm
Copy link
Member

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.

@findleyr
Copy link
Contributor

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.

@gopherbot
Copy link

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

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

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

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>
@golang golang locked and limited conversation to collaborators Sep 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge generics Issue is related to generics NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants