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: can't specify value for blank fields of array types. #38690

Closed
go101 opened this issue Apr 27, 2020 · 8 comments
Closed

cmd/compile: can't specify value for blank fields of array types. #38690

go101 opened this issue Apr 27, 2020 · 8 comments
Labels
Milestone

Comments

@go101
Copy link

@go101 go101 commented Apr 27, 2020

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

$ go version
go version go1.14.2 linux/amd64

Does this issue reproduce with the latest release?

Yes

What did you do?

package main

import "fmt"

func cmpS(a, b S) {
	defer func() {
		if recover() != nil {
			fmt.Println("panic")
		}
	}()
	fmt.Println(a == b)
}

type S struct{x int; _ [2]int}

func main() {
	cmpS(S{1, [2]int{2}}, S{1, [2]int{3}}) // cannot use _ as value
}

What did you expect to see?

Compiles ok.

What did you see instead?

Failed to compile.

BTW, it compiles ok with gccgo.

@go101 go101 changed the title cmd/compile: can't specified value for blank fields of array types. cmd/compile: can't specify value for blank fields of array types. Apr 27, 2020
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 27, 2020

Change https://golang.org/cl/230121 mentions this issue: cmd/compile: fix literal value can't be used in struct blank field of type array

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 27, 2020

The error message looks wrong but it's not obvious to me that the program should work. Is a composite literal allowed to set the value of a _ field?

@ianlancetaylor ianlancetaylor modified the milestones: Unplanned, Backlog Apr 27, 2020
@mdempsky
Copy link
Member

@mdempsky mdempsky commented Apr 28, 2020

We allow initializing blank, non-array-typed fields in non-keyed composite literals, so we should probably allow blank, array-typed fields too.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 28, 2020

OK, SGTM.

@go101
Copy link
Author

@go101 go101 commented Apr 28, 2020

It looks struct literals also have this problem, but slice literals not.
And not all such array/struct literals have this problem.
It looks this problem only happens in binary operator operations.

package main


type T = struct{y int} // struct literals also fail to compile
//type T = [1]int
//type T = []int // slice literals are ok
type S struct{x int; _ T}

var x, y = S{1, T{2}}, S{1, T{2}} // ok

func main() {
	println(S{1, T{2}}.x == S{1, T{3}}.x) // cannot use _ as value
}
@josharian
Copy link
Contributor

@josharian josharian commented Apr 28, 2020

Does it also only happen in structs with <= 4 fields? If so, the bug might be in the inlined struct comparison code in walkCompare in walk.go.

@cuonglm
Copy link
Contributor

@cuonglm cuonglm commented Apr 28, 2020

Does it also only happen in structs with <= 4 fields? If so, the bug might be in the inlined struct comparison code in walkCompare in walk.go.

Seems that involves alias type, using struct directly compiling ok.

@cuonglm
Copy link
Contributor

@cuonglm cuonglm commented Apr 28, 2020

Does it also only happen in structs with <= 4 fields? If so, the bug might be in the inlined struct comparison code in walkCompare in walk.go.

Seems that involves alias type, using struct directly compiling ok.

Ah no, so it works with 0-field struct.

package main

import "fmt"

func cmpS(a, b S) {
	defer func() {
		if recover() != nil {
			fmt.Println("panic")
		}
	}()
	fmt.Println(a == b)
}

type T struct{}
type S struct{x int; _ T}

func main() {
	cmpS(S{1, T{}}, S{1, T{}}) // compile ok
}

It's also ok with [0]int.

@gopherbot gopherbot closed this in 0f47c12 May 6, 2020
xujianhai666 added a commit to xujianhai666/go-1 that referenced this issue May 21, 2020
Fixes golang#38690

Change-Id: I3544daf617fddc0f89636265c113001178d16b0c
Reviewed-on: https://go-review.googlesource.com/c/go/+/230121
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@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
7 participants
You can’t perform that action at this time.