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: pack structs containing anonymous fields more tightly #31047

Open
josharian opened this issue Mar 26, 2019 · 4 comments

Comments

@josharian
Copy link
Contributor

commented Mar 26, 2019

I attempted to refactor some common fields (E) out of a struct (T1), yielding (T2). Unfortunately, this changed the packing of the fields, which makes this refactoring infeasible in this case.

type E struct {
	X int
	B uint8
}

type T1 struct {
	X int
	B uint8
	C uint8
}

type T2 struct {
	E
	C uint8
}

Observe the Offset of C in this code: https://play.golang.com/p/ac5CCIWIlZS

On a first pass, I don't see anything in the spec that forbids T1 and T2 being laid out identically in memory.

@tmthrgd

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

Would that not break the following?

var t T2
e := &t.E
*e = E{}
@josharian

This comment has been minimized.

Copy link
Contributor Author

commented Mar 27, 2019

I don’t follow. Why would it?

@randall77

This comment has been minimized.

Copy link
Contributor

commented Mar 27, 2019

Right now E.Size() is 16 (on 64-bit archs). We would need E.Size() to be 9 for this proposal to work.
But an array of E would still need to be strided by 16, so we'd need to introduce the concept of two different kinds of size for each type.

@tmthrgd

This comment has been minimized.

Copy link
Contributor

commented Mar 28, 2019

Consider a slightly more involved example:

func main() {
    var t T2
    t.C = 42
    modifyE(&t.E)
    println(t.C)
}

//go:noinline
func modifyE(e *E) {
    *e = E{}
}

modifyE would zero 8/16 bytes and override the C field. Keep in mind that function could exist in a different package or *E could be passed through as an interface.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.