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: allows assigning to blank identifier in struct #31546

Closed
bradfitz opened this issue Apr 18, 2019 · 6 comments
Closed

cmd/compile: allows assigning to blank identifier in struct #31546

bradfitz opened this issue Apr 18, 2019 · 6 comments
Labels
Milestone

Comments

@bradfitz
Copy link
Member

@bradfitz bradfitz commented Apr 18, 2019

https://play.golang.org/p/WMC4t5oiRR-

package main

import (
	"fmt"
	"reflect"
)

var x = struct{ a, _, c int }{1, 2, 3}

func main() {
	var y = struct{ a, _, c int }{1, 2, 3}
	fmt.Println(x)
	fmt.Println(y)
	fmt.Println(reflect.ValueOf(x).Field(1).Int())
	fmt.Println(reflect.ValueOf(y).Field(1).Int())
	fmt.Println(x == y)
}

Currently (Go 1.12, master) says:

{1 2 3}
{1 0 3}
2
0
true

The x == y part returning true is defined by the spec, but x and y getting a different value for the blank identifier is... weird. I see nothing in the spec that addresses this directly but if squint I might be able to connect some different parts and make guesses as to intent.

But Go 1.4 - Go 1.8 fail to compile with:

./x.go:11: cannot refer to blank field or method

Go 1.9 is the first version where the program above compiles and returns that result.

/cc @ianlancetaylor @griesemer @josharian @cherrymui @randall77 @davecheney

@bradfitz bradfitz added this to the Go1.13 milestone Apr 18, 2019
@cuonglm

This comment has been minimized.

Copy link
Contributor

@cuonglm cuonglm commented Apr 18, 2019

it seems to be a regression of https://go-review.googlesource.com/c/go/+/38006/

@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Apr 18, 2019

@cuonglm the discussion in #19482 (which that CL fixes) indicates that the previous behavior is wrong. But maybe I'm misinterpreting what the "it" is that was the regression.

There are some possibly related cases in #15481.

x and y getting a different value for the blank identifier is... weird.

This seems to be the crux of the issue. I think we need to start with some clarity about what the correct behavior is here. (Or whether it is ok for the value of the blank field to be implementation dependent.)

@jimmyfrasche

This comment has been minimized.

Copy link
Member

@jimmyfrasche jimmyfrasche commented Apr 18, 2019

If you told me one was legal and asked me what I expect to happen without looking at the spec or testing it in the playground, I would say y by analogy with

var a, _, c int = 1, 2, 3

which is effectively equivalent to

var a, c = 1, 3

But I also think that https://play.golang.org/p/Qzcyybi8ZUV should print 0 by the same logic, so maybe I'm not the best person to ask 😄

@rsc

This comment has been minimized.

Copy link
Contributor

@rsc rsc commented Apr 24, 2019

The reason there's any assignment allowed at all is for:

type T struct {x, _, z int}
var t1, t2 T
t1 = t2

where the _ is padding.

The literal assignment fell out from changing the rules to allow the whole-struct assignment but was probably a mistake. If we had it to do over again, we could probably disallow:

var x = struct{ a, _, c int }{1, 2, 3}

But we don't have it to do over again, and it is silly to insist on writing the _ field, so it seems like we must insist on not writing it during the literal. So the global is the wrong behavior that needs fixing.

@josharian

This comment has been minimized.

Copy link
Contributor

@josharian josharian commented Apr 25, 2019

Working on a fix.

@gopherbot

This comment has been minimized.

Copy link

@gopherbot gopherbot commented Apr 25, 2019

Change https://golang.org/cl/173723 mentions this issue: cmd/compile: don't initialize blank struct fields

@gopherbot gopherbot closed this in 2693b42 Apr 25, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
6 participants
You can’t perform that action at this time.