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/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
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Milestone
Go1.13

Comments

@bradfitz
Copy link
Contributor

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 the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 18, 2019
@bradfitz bradfitz added this to the Go1.13 milestone Apr 18, 2019
@cuonglm
Copy link
Member

cuonglm commented Apr 18, 2019

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

@josharian
Copy link
Contributor

josharian commented Apr 18, 2019 •
edited

@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
Copy link
Member

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 😄

@bradfitz bradfitz mentioned this issue Apr 18, 2019
Closed
@rsc
Copy link
Contributor

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.

@bradfitz bradfitz added the NeedsFix The path to resolution is known, but the work has not been done. label Apr 24, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Apr 24, 2019
@josharian
Copy link
Contributor

Working on a fix.

@gopherbot
Copy link

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

@golang golang locked and limited conversation to collaborators Apr 24, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Milestone
Go1.13
Development

No branches or pull requests
6 participants
@bradfitz @josharian @rsc @jimmyfrasche @cuonglm @gopherbot