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: "invalid recursive type" should abort #21273

Closed
bep opened this issue Aug 2, 2017 · 7 comments
Closed

cmd/compile: "invalid recursive type" should abort #21273

bep opened this issue Aug 2, 2017 · 7 comments
Assignees
Milestone

Comments

@bep
Copy link
Contributor

@bep bep commented Aug 2, 2017

https://play.golang.org/p/F3pxiyBuIL

package main

type (
	string string
	a      struct {
		m map[string]int
	}
)

The above fails with

./main.go:4: invalid recursive type string
./main.go:6: invalid map key type string

Which is easy to understand and fix. When I recently did a bad "search and replace" in a large code base I ended up with

 invalid map key type string
 invalid map key type string (lots of these)
...
... too many errors

Which is harder to debug without seeing the origin of the error. It will be easier for me to find the culprit the next time I experience something like this, but I had one hour of scratching my head feeling stupid.

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Aug 2, 2017

I'm not sure I understand this report.

The second paste has the usual too many errors line, which means that only a few errors were reported, is that right? Wasn't the first one invalid recursive type string? Or was that error line missing? Or it was there and you didn't past it? Also, there were no line numbers?

Can you post a reproducer that actually triggers the behaviour that you found "harder to debug"?

@bep

This comment has been minimized.

Copy link
Contributor Author

@bep bep commented Aug 2, 2017

The full output was:

▶ go build
# github.com/gohugoio/hugo/hugolib
hugolib/menu.go:45: invalid map key type string
hugolib/multilingual.go:34: invalid map key type string
hugolib/page.go:97: invalid map key type string
hugolib/shortcode.go:184: invalid map key type scKey
hugolib/shortcode.go:188: invalid map key type scKey
hugolib/shortcode.go:193: invalid map key type string
hugolib/shortcode.go:196: invalid map key type string
hugolib/shortcode.go:199: invalid map key type string
hugolib/taxonomy.go:23: invalid map key type string
hugolib/taxonomy.go:33: invalid map key type string
hugolib/taxonomy.go:23: too many errors

I can update my original description to make it clearer if you want (I chose not to, as that would make your reply look out of place).

I can add to this that if this was a type definition deep down in some vendored lib, I would probably never have found it ...

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Aug 2, 2017

Thanks for clarifying. It's clear that the current output is not ideal because the fact that the invalid recursive type string error -cause of all the other errors- wasn't printed makes really hard to debug this kind of issue.

@ALTree

This comment has been minimized.

Copy link
Member

@ALTree ALTree commented Aug 2, 2017

Here's a one-file reproducer:

package main

type string string

type a struct {
	m map[string]int
}

type b1 map[string]int
type b2 map[string]int
type b3 map[string]int
type b4 map[string]int
type b5 map[string]int
type b6 map[string]int
type b7 map[string]int
type b8 map[string]int
type b9 map[string]int

Output:

tmp/sandbox809932791/main.go:6: invalid map key type string
tmp/sandbox809932791/main.go:9: invalid map key type string
tmp/sandbox809932791/main.go:10: invalid map key type string
tmp/sandbox809932791/main.go:11: invalid map key type string
tmp/sandbox809932791/main.go:12: invalid map key type string
tmp/sandbox809932791/main.go:13: invalid map key type string
tmp/sandbox809932791/main.go:14: invalid map key type string
tmp/sandbox809932791/main.go:15: invalid map key type string
tmp/sandbox809932791/main.go:16: invalid map key type string
tmp/sandbox809932791/main.go:17: invalid map key type string
tmp/sandbox809932791/main.go:17: too many errors

If you comment one of the type bX line, you'll also get (in first position), the error that points to the actual culprit:

tmp/sandbox056532838/main.go:3: invalid recursive type string

Choosing which errors to print (or to abort in specific places) is not easy, I'm not sure if it's even possible to choose a general policy that will work well in every case.

@ALTree ALTree added this to the Go1.10 milestone Aug 2, 2017
@ALTree ALTree added the NeedsDecision label Aug 2, 2017
@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Oct 30, 2017

Better repro case:

r$ go tool compile /tmp/x.go
/tmp/x.go:4:2: invalid recursive type T
/tmp/x.go:6:5: invalid map key type T
r$ cat /tmp/x.go
package main

type (
	T T
	a      struct {
		m map[T]int
	}
)
r$ 

There's no need to tell me T is an invalid map key after telling me it's an invalid anything.

@griesemer

This comment has been minimized.

Copy link
Contributor

@griesemer griesemer commented Nov 1, 2017

Likely related, this variation leads to a compiler stack overflow:

package p

type (
	T struct { T }
	_ map[T]int
)

(which happens to be #21657)

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Nov 2, 2017

Change https://golang.org/cl/75310 mentions this issue: cmd/compile: avoid spurious errors for invalid map key types

@gopherbot gopherbot closed this in 25159d3 Nov 2, 2017
@golang golang locked and limited conversation to collaborators Nov 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants
You can’t perform that action at this time.