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

Jump to bottom

cmd/go2go: Translation fails when inferring a aliased generic type #39797

Closed
firelizzard18 opened this issue Jun 24, 2020 · 11 comments
Closed

cmd/go2go: Translation fails when inferring a aliased generic type #39797

firelizzard18 opened this issue Jun 24, 2020 · 11 comments
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone
Unreleased

Comments

@firelizzard18
Copy link
Contributor

If package a defines type S as a generic type, package b defines type S = b.S, if a expression of type S is passed to a generic function without specifying the function's type parameters (relying on inference), go tool go2go build will fail in a peculiar way. It does not fail if the type parameter is passed explicitly.

package a

type S(type T) struct {
	F T
}
package b

import "./a"

type S = a.S

func F1(type T)(T) { }

// fails
func F2() { F1(S(string){}) }

// compiles
func F3() { F1(S(string))(S(string){}) }

Replacing return err with panic(err) on rewrite.go:361 in rewriteAST produces the following:

panic: cannot find package "a" in any of:
        /usr/lib/go-tip/src/a (from $GOROOT)
        /home/firelizzard/go/src/a (from $GOPATH)

goroutine 1 [running]:
go/go2go.rewriteAST(0xc00010c240, 0xc000024420, 0x0, 0x0, 0xc0000714f0, 0xc00011c180, 0x7f44c7755201, 0xc00010c2c0, 0x816bc0)
        /usr/lib/go-tip/src/go/go2go/rewrite.go:361 +0x23b5
go/go2go.rewriteFile(0x693853, 0x1, 0xc00010c240, 0xc000024420, 0x0, 0x0, 0xc0000714f0, 0xc000012438, 0x6, 0xc00011c180, ...)
        /usr/lib/go-tip/src/go/go2go/rewrite.go:221 +0xb8
go/go2go.rewriteFilesInPath(0xc000024420, 0x0, 0x0, 0x693853, 0x1, 0xc00000c1c0, 0x1, 0x2, 0x0, 0x0, ...)
        /usr/lib/go-tip/src/go/go2go/go2go.go:104 +0xc11
go/go2go.rewriteToPkgs(0xc000024420, 0x0, 0x0, 0x693853, 0x1, 0xc00000c1a0, 0xc0000748d0, 0xc0000748a0, 0xc000074870, 0xc000074840)
        /usr/lib/go-tip/src/go/go2go/go2go.go:46 +0x165
go/go2go.Rewrite(...)
        /usr/lib/go-tip/src/go/go2go/go2go.go:30
main.translate(0xc000024420, 0x693853, 0x1)
        /usr/lib/go-tip/src/cmd/go2go/translate.go:15 +0x47
main.main()
        /usr/lib/go-tip/src/cmd/go2go/main.go:69 +0x9ec
exit status 2

During *translator.translate, *translator.addTypePackages adds typ.Obj().Pkg() to t.typePackages, which is to generate a list of import statements. Because typ.Obj().Pkg().Path() only includes the package name ("a") and not the full path, the generated import statement is just import "a", and that causes problems. This can eventually be traced back to the fact that go/go2go.parseFiles uses the package <name> as the package path instead of a full path.

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

$ go version
go version devel +da7932368b Mon Jun 22 19:06:44 2020 +0000 linux/amd64

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

go env Output
$ go env
GOARCH="amd64"
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOOS="linux"

What did you do?

  • git clone https://gitlab.com/firelizzard/go-iter
  • cd go-iter/issues/i2
  • go tool go2go build

What did you expect to see?

A successful build

What did you see instead?

cannot find package "a" in any of:
        /usr/lib/go-tip/src/a (from $GOROOT)
        /home/REDACTED/go/src/a (from $GOPATH)
@firelizzard18 firelizzard18 mentioned this issue Jun 24, 2020
Closed
@firelizzard18
Copy link
Contributor Author

The following fails with the same translator error:

package i2

import "./a"

type S = a.S

type T(type A, B) func(A) B

func F1(type T)(t T) { }

func (tr T(A, B)) M() { F1(*new(B)) }

func F2() { _ = T(string, S(string))(nil) }

@cagedmantis cagedmantis added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 24, 2020
@cagedmantis cagedmantis added this to the Unreleased milestone Jun 24, 2020
@cagedmantis
Copy link
Contributor

/cc @griesemer @ianlancetaylor

@ianlancetaylor
Copy link
Contributor

What are your GOPATH and GO2PATH environment variables set to?

This is what happens for me. GO2PATH is not set. This is slightly different from what you are reporting.

> git clone https://gitlab.com/firelizzard/go-iter
Cloning into 'go-iter'...
warning: redirecting to https://gitlab.com/firelizzard/go-iter.git/
remote: Enumerating objects: 145, done.
remote: Counting objects: 100% (145/145), done.
remote: Compressing objects: 100% (70/70), done.
remote: Total 145 (delta 65), reused 137 (delta 57), pack-reused 0
Receiving objects: 100% (145/145), 18.94 KiB | 3.79 MiB/s, done.
Resolving deltas: 100% (65/65), done.
> cd go-iter/issues/i2
> go tool go2go build
type checking failed for i2
i2.go2:3:8: could not import gitlab.com/firelizzard/go-iter/issues/i2/a (cannot find package "gitlab.com/firelizzard/go-iter/issues/i2/a" in any of:
	/home/iant/gogen/src/gitlab.com/firelizzard/go-iter/issues/i2/a (from $GOROOT)
	/home/iant/gopath/src/gitlab.com/firelizzard/go-iter/issues/i2/a (from $GOPATH))

@firelizzard18
Copy link
Contributor Author

GOPATH is the default (/home/firelizzard/go) and GO2PATH is /usr/src/go. I get same error as you if I unset GO2PATH.

@gopherbot
Copy link

Change https://golang.org/cl/239703 mentions this issue: [dev.go2go] go/go2go: don't import both "./a" and "a"

@ianlancetaylor
Copy link
Contributor

I've committed a patch to the dev.go2go branch that affects how dot imports are handled. But I don't know whether it fixes your test case.

gopherbot pushed a commit that referenced this issue Jun 24, 2020
@ianlancetaylor
[dev.go2go] go/go2go: don't import both "./a" and "a"
af16dac 
For #39797

Change-Id: I4063a0138062ce8dc1f35cc7fef7c6773c939362
Reviewed-on: https://go-review.googlesource.com/c/go/+/239703
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@ianlancetaylor
Copy link
Contributor

OK, thanks. I'll leave this open but I don't plan to fix it. My main interest for the go2go tool to support import "./a" is to support the tests in test/gen. Making relative imports work in general looks kind of painful. The go2go tool is a temporary tool for experimenting with generics; it doesn't need to handle all cases. If you want to send a patch for this, I'll look at it. Sorry.

@firelizzard18
Copy link
Contributor Author

That's what's weird, I'm not using relative imports. I put `import "./a" in the example above to simplify it, but if you look at gitlab.com/firelizzard/go-iter/i2/i2.go2, the import is absolute.

Somewhere in the translation, gitlab.com/firelizzard/go-iter/issues/i2/a becomes a (absolute, not relative). As far as I can tell, import "a" is added to the generated code (except I've never seen that on disk), and it fails to compile because "a" does not exist.

@gopherbot
Copy link

Change https://golang.org/cl/239707 mentions this issue: [dev.go2go] go/go2go: if we have an import path, pass it to go/types

@ianlancetaylor
Copy link
Contributor

OK, I think I see. Because the file imports a gitlab.com path, this has to be under $GO2PATH/src/gitlab.com/firelizzard/go-iter. I just committed a fix that makes that case work for me. Closing, but let me know if you still have trouble.

gopherbot pushed a commit that referenced this issue Jun 24, 2020
@ianlancetaylor
[dev.go2go] go/go2go: if we have an import path, pass it to go/types
2dce748 
No test because it doesn't really fit into the current test harness.

FIxes #39797

Change-Id: I410217a0e6b3af6808a111d0edbafaa3047b12f1
Reviewed-on: https://go-review.googlesource.com/c/go/+/239707
Reviewed-by: Ian Lance Taylor <iant@golang.org>
@firelizzard18
Copy link
Contributor Author

That works, thanks!

@golang golang locked and limited conversation to collaborators Jun 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@cagedmantis @firelizzard18 @ianlancetaylor @gopherbot