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: compiler crash with "Dictionary should have already been generated" #52279

Closed
benjivesterby opened this issue Apr 11, 2022 · 10 comments
Closed
Labels
NeedsInvestigation release-blocker
Milestone

Comments

@benjivesterby
Copy link

@benjivesterby benjivesterby commented Apr 11, 2022

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

1.18

Does this issue reproduce with the latest release?

Yes

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

GO111MODULE=""
GOARCH="amd64"
GOBIN="/home/benji/go/bin"
GOCACHE="/home/benji/.cache/go-build"
GOENV="/home/benji/.config/go/env"
GOEXE=""
GOEXPERIMENT=""
GOFLAGS=""
GOHOSTARCH="amd64"
GOHOSTOS="linux"
GOINSECURE=""
GOMODCACHE="/home/benji/go/pkg/mod"
GONOPROXY=""
GONOSUMDB=""
GOOS="linux"
GOPATH="/home/benji/go"
GOPRIVATE=""
GOPROXY="https://proxy.golang.org,direct"
GOROOT="/usr/local/go"
GOSUMDB="sum.golang.org"
GOTMPDIR=""
GOTOOLDIR="/usr/local/go/pkg/tool/linux_amd64"
GOVCS=""
GOVERSION="go1.18"
GCCGO="gccgo"
GOAMD64="v1"
AR="ar"
CC="gcc"
CXX="g++"
CGO_ENABLED="1"
GOMOD="/home/benji/src/test/bug/go.mod"
GOWORK=""
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-build616750673=/tmp/go-build -gno-record-gcc-switches"

What did you do?

Tried using this generic method go.structs.dev/gen/FMap.Flip() cross-package and received the following error:

benji | bug $ go build ./...
# go.structs.dev/gen/bug/imp
<autogenerated>:1: internal compiler error: Dictionary should have already been generated: go.structs.dev/gen..dict.FMap[string,go.structs.dev/gen/bug/lib.MyType].Flip

Please file a bug report including a short program that triggers the error.
https://go.dev/issue/new

Here is a complete reproducible version:

bug.tar.gz

benji | bug $ tree
.
├── go.mod
├── go.sum
├── imp
│   └── imp.go
└── lib
    └── lib.go

2 directories, 4 files
// lib/lib.go
package lib

import (
	"fmt"
	"strings"

	. "go.structs.dev/gen"
)

type MyType uint8

const (
	FIRST MyType = iota
	SECOND
	THIRD
)

var typeStrs = FMap[MyType, string]{
	FIRST:  "FIRST",
	SECOND: "SECOND",
	THIRD:  "THIRD",
}

var typeStrsR = typeStrs.Flip()

func (self MyType) String() string {
	str, exists := typeStrs[self]
	if !exists {
		return "UNKNOWN"
	}

	return str
}

func ParseMyType(t string) (MyType, error) {
	tpe, exists := typeStrsR[strings.ToUpper(t)]
	if !exists {
		return 0, fmt.Errorf("invalid type: %s", t)
	}
	return tpe, nil
}
// imp/imp.go
package imp

import "go.structs.dev/gen/bug/lib"

func Print() {

	lib.FIRST.String()
}

What did you expect to see?

I expected it to compile and be able to use the flipped map as a normal type aliased go.structs.dev/gen/Map

What did you see instead?

Got this error:

benji | bug $ go build ./...
# go.structs.dev/gen/bug/imp
<autogenerated>:1: internal compiler error: Dictionary should have already been generated: go.structs.dev/gen..dict.FMap[string,go.structs.dev/gen/bug/lib.MyType].Flip

Please file a bug report including a short program that triggers the error.
https://go.dev/issue/new
@benjivesterby benjivesterby changed the title affected/package: generics - internal compiler error: Dictionary should have already been generated compiler/generics - internal compiler error: Dictionary should have already been generated Apr 11, 2022
@ianlancetaylor ianlancetaylor changed the title compiler/generics - internal compiler error: Dictionary should have already been generated cmd/compile: compiler crash with "Dictionary should have already been generated" Apr 11, 2022
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 11, 2022

A simpler version of lib.go that doesn't require an import but still recreates the problem:

// lib/lib.go
package lib

type Map[K comparable, V any] map[K]V

type FMap[K, V comparable] Map[K, V]

func (m FMap[K, V]) Flip() FMap[V, K] {
	out := make(FMap[V, K])
	for k, v := range m {
		out[v] = k
	}

	return out
}

type MyType uint8

const (
	FIRST MyType = iota
)

var typeStrs = FMap[MyType, string]{
	FIRST:  "FIRST",
}

func (self MyType) String() string {
	str, exists := typeStrs[self]
	if !exists {
		return "UNKNOWN"
	}

	return str
}

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 11, 2022

CC @randall77 @mdempsky

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 11, 2022

@gopherbot Please open a backport to 1.18

This looks like a compiler crash on valid code.

@ianlancetaylor ianlancetaylor added NeedsInvestigation release-blocker labels Apr 11, 2022
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 11, 2022

Backport issue(s) opened: #52286 (for 1.18).

Remember to create the cherry-pick CL(s) as soon as the patch is submitted to master, according to https://go.dev/wiki/MinorReleases.

@ianlancetaylor ianlancetaylor added this to the Go1.19 milestone Apr 11, 2022
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 11, 2022

Repro on playground: https://go.dev/play/p/lFHvAeGwc1g?v=gotip

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Apr 12, 2022

I'm looking at this, as part of my "learn generics by playing with bugs", and I see (with two debugging booleans set and -W=2)

# bug/lib
...
=== Finalized dictionary .dict.FMap[string,"".MyType].Flip

Where the missing dictionary is bug/lib..dict.FMap[string,bug/lib.MyType].Flip.

Do I read this correctly as an export/import problem?

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented Apr 14, 2022

Still slogging, intermittent with other work, but with gdlv on the failed compile, it is compiling
(*FMap[string,bug/lib.MyType]).Flip
-- so this is an an export problem (?), for the initial export (and not, say, a re-export of something in bug with inlining). Perhaps, actually a naming problem, in some way similar to https://go-review.googlesource.com/c/go/+/391574 ?

Error messages suggest however that I am compiling package "bug" and that perhaps the value of the current function is stale?

The bug does not reproduce when inlining is turned off.
The bug does not reproduce with GOEXPERIMENT=unified.

@dr2chase
Copy link
Contributor

@dr2chase dr2chase commented May 9, 2022

Further work. A simpler version of lib/lib.go:

// lib/lib.go
package lib

type FMap[K comparable, V comparable] map[K]V

//go:noinline
func (m FMap[K, V]) Flip() FMap[V, K] {
	out := make(FMap[V, K])
	return out
}

type MyType uint8

const (
	FIRST MyType = 0
)

var typeStrs = FMap[MyType, string]{
	FIRST: "FIRST",
}

func (self MyType) String() string {
	return typeStrs[self]
}

// func F() FMap[string, MyType] {
// 	return typeStrs.Flip()
// }

Uncomment F at the bottom to make it pass; I think this is an export problem, where the exported inline for MyType.String() includes a reference to typeStrs (type FMap[MyType, string]) which in turn has a method Flip() which mentions the type FMap[string, MyType] which also has a method Flip. I haven't figured out where/how this is all supposed to be exported, only that it is not.

@gopherbot
Copy link

@gopherbot gopherbot commented May 10, 2022

Change https://go.dev/cl/405118 mentions this issue: cmd/compile: be sure to export types mentioned in f.i.g. method signature

@gopherbot
Copy link

@gopherbot gopherbot commented May 11, 2022

Change https://go.dev/cl/405543 mentions this issue: cmd/compile: be sure to export types mentioned in f.i.g. method signature

gopherbot pushed a commit that referenced this issue May 17, 2022
…ture

When a fully instantiated generic method is exported, be sure to also
export the types in its signature.

Updates #52279.
Fixes #52286.

Change-Id: Icc6bca05b01f914cf67faaf1bf184eaa5484f521
Reviewed-on: https://go-review.googlesource.com/c/go/+/405118
Run-TryBot: David Chase <drchase@google.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
(cherry picked from commit 1284cc2)
Reviewed-on: https://go-review.googlesource.com/c/go/+/405543
Reviewed-by: Keith Randall <khr@golang.org>
Reviewed-by: Keith Randall <khr@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation release-blocker
Projects
None yet
Development

No branches or pull requests

5 participants