code review 7363054: go/types: fix sizeof computations

go/types: fix sizeof computations

Context.Alignof/Offsetsof/Sizeof now provide means
to customize the type checker for a given platform.

- provide Context.Offsetsof to specify the
  offsets of struct fields
- use the correct sizes for ints, uint, uintptrs
  in constant computations
- moved all size computations into separate file
  (sizes.go)
- fixed a bug with string constant slicing

[gri, 2013-02-25 22:36:25]

Hello adonovan@google.com, axwalk@gmail.com (cc: golang-dev@googlegroups.com),

I'd like you to review this change to
https://code.google.com/p/go

[adonovan, 2013-02-26 04:56:49]

LGTM

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go#n...
src/pkg/go/types/types.go:125: offsets []int64 // field offsets in bytes, lazily
computde
"computed"

[axw, 2013-02-26 05:02:09]

A few quibbles, but otherwise LGTM.

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/api.go
File src/pkg/go/types/api.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/api.go#new...
src/pkg/go/types/api.go:64: // of the given struct fields, in bytes. Otherwise
DefaultOffsetof
s/DefaultOffsetof/DefaultOffsetsof/

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/builtins.go
File src/pkg/go/types/builtins.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/builtins.g...
src/pkg/go/types/builtins.go:343: check.invalidArg(x.pos(), "field %s is
embedded via a pointer in %s", sel, x)
This makes sense, but gc allows this. gc seems to be giving the offset relative
to the embedded field's address; bug?

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/sizes.go
File src/pkg/go/types/sizes.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/sizes.go#n...
src/pkg/go/types/sizes.go:111: // DefaultOffsetsof is implements the default
field offset computation
s/is implements/implements/

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/sizes.go#n...
src/pkg/go/types/sizes.go:141: return DefaultSizeof(t.Elt) * t.Len // may be 0
Is DefaultSizeof expected to operate the same as gc? If so, there should be
padding here.

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go#n...
src/pkg/go/types/types.go:125: offsets []int64 // field offsets in bytes, lazily
computde
s/computde/computed/

[gri, 2013-02-26 06:00:12]

Thanks for the careful review.
- gri

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/api.go
File src/pkg/go/types/api.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/api.go#new...
src/pkg/go/types/api.go:64: // of the given struct fields, in bytes. Otherwise
DefaultOffsetof
On 2013/02/26 05:02:09, axw wrote:
> s/DefaultOffsetof/DefaultOffsetsof/

Done.

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/builtins.go
File src/pkg/go/types/builtins.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/builtins.g...
src/pkg/go/types/builtins.go:343: check.invalidArg(x.pos(), "field %s is
embedded via a pointer in %s", sel, x)
On 2013/02/26 05:02:09, axw wrote:
> This makes sense, but gc allows this. gc seems to be giving the offset
relative
> to the embedded field's address; bug?

Yes, it's a gc bug - filed today: issue 4909; see also issues 4905, 4911

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/sizes.go
File src/pkg/go/types/sizes.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/sizes.go#n...
src/pkg/go/types/sizes.go:111: // DefaultOffsetsof is implements the default
field offset computation
On 2013/02/26 05:02:09, axw wrote:
> s/is implements/implements/

Done.

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/sizes.go#n...
src/pkg/go/types/sizes.go:141: return DefaultSizeof(t.Elt) * t.Len // may be 0
On 2013/02/26 05:02:09, axw wrote:
> Is DefaultSizeof expected to operate the same as gc? If so, there should be
> padding here.

It's not, but I'll add alignment of the elements; makes sense.

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go
File src/pkg/go/types/types.go (right):

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go#n...
src/pkg/go/types/types.go:125: offsets []int64 // field offsets in bytes, lazily
computde
On 2013/02/26 05:02:09, axw wrote:
> s/computde/computed/

Done.

https://codereview.appspot.com/7363054/diff/17001/src/pkg/go/types/types.go#n...
src/pkg/go/types/types.go:125: offsets []int64 // field offsets in bytes, lazily
computde
On 2013/02/26 05:02:09, axw wrote:
> s/computde/computed/

Done.

[gri, 2013-02-26 06:07:13]

*** Submitted as https://code.google.com/p/go/source/detail?r=512627a9cdb8 ***

go/types: fix sizeof computations

Context.Alignof/Offsetsof/Sizeof now provide means
to customize the type checker for a given platform.

- provide Context.Offsetsof to specify the
  offsets of struct fields
- use the correct sizes for ints, uint, uintptrs
  in constant computations
- moved all size computations into separate file
  (sizes.go)
- fixed a bug with string constant slicing

R=adonovan, axwalk
CC=golang-dev
https://codereview.appspot.com/7363054