code review 6899045: spec: constant indexes and array/slice sizes must fit i...

spec: constant indexes and array/slice sizes must fit into int64

also:
- consistently use 'indexes' rather than 'indices'

This CL refines earlier CLs 6725053 and 6699048.
Restricting the maximum value for integer indices
to 1<<63-1 ("MaxInt64") is reasonable and permits
more compile-time checks (and a simpler implementation).

The compilers have implemented similar restrictions
early on, so this is unlikely to cause existing
programs to break.

[gri, 2012-12-06 21:10:30]

Hello rsc@golang.org, iant@golang.org, r@golang.org, ken@golang.org (cc:
golang-dev@googlegroups.com),

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

[r, 2012-12-06 21:30:04]

The plural of index in a mathematical context is 'indices'. The plural
of index in a catalog context is 'indexes'. The correct plural in this
document is 'indices'. I have said this before.

That aside, I'm unsure about this change. First question: Why int64
rather than int? After all, addressing only goes up to the maximum
positive value representable in an int.

You're missing a semicolon from one of your <s.

-rob

[rsc, 2012-12-06 21:37:18]

We have been going back and forth on indices and indexes. Indexes is a
common alternative spelling, matches the heading in the spec, and is
found when you search for index.

The rule used to be 'has to fit in int', but that means x[1<<40]
compiles on amd64 but does not compile on 386. We were trying to make
whether a Go program is valid not depend on the architecture (at least
for this specific rule), so an earlier CL removed the limit entirely.
However, we decided that allowing indexes that didn't work on _any_
architecture was silly, so this CL re-restricts the indexes, this time
to fit in an int64, for consistency across architectures.

Russ

[remyoudompheng, 2012-12-06 21:43:53]

On 2012/12/6 Russ Cox <rsc@golang.org> wrote:
> We have been going back and forth on indices and indexes. Indexes is a
> common alternative spelling, matches the heading in the spec, and is
> found when you search for index.
>
> The rule used to be 'has to fit in int', but that means x[1<<40]
> compiles on amd64 but does not compile on 386. We were trying to make
> whether a Go program is valid not depend on the architecture (at least
> for this specific rule), so an earlier CL removed the limit entirely.
> However, we decided that allowing indexes that didn't work on _any_
> architecture was silly, so this CL re-restricts the indexes, this time
> to fit in an int64, for consistency across architectures.

But nobody is trying to make:

package main
import "fmt"
func main() {
	x := 0x123456789abcdef
	fmt.Println(x)
}

compile regardless of architecture, right?

Rémy.

[rsc, 2012-12-06 22:19:25]

> But nobody is trying to make:
>
> package main
> import "fmt"
> func main() {
>         x := 0x123456789abcdef
>         fmt.Println(x)
> }
>
> compile regardless of architecture, right?

No.

[rsc, 2012-12-06 22:19:42]

(Right.)

[r, 2012-12-07 04:45:14]

Index is a Latin-rooted word with a Latin plural. To use 'indexes'
rather than 'indices' in this context isn't as bad as 'mediums' for
'media' or 'semantics' as a plural, and it (all of them, really) may
well be the way English is going, but it bothers me to use a word we
know is, however mildly, incorrect.

That trivium (ha!) aside, what is the right thing to do about the
compile-time limits on an index constant? I don't see why 'int' isn't
the right answer.

-rob

[iant2, 2012-12-07 05:09:42]

On Thu, Dec 6, 2012 at 8:45 PM, Rob Pike <r@golang.org> wrote:
> Index is a Latin-rooted word with a Latin plural. To use 'indexes'
> rather than 'indices' in this context isn't as bad as 'mediums' for
> 'media' or 'semantics' as a plural, and it (all of them, really) may
> well be the way English is going, but it bothers me to use a word we
> know is, however mildly, incorrect.

This issue is both flammable and inflammable.

> That trivium (ha!) aside, what is the right thing to do about the
> compile-time limits on an index constant? I don't see why 'int' isn't
> the right answer.

It's occasionally nice to be able to write code like

    var big []int
    if unsafe.Sizeof(int(1)) == 4 {
        big = make([]int, 1 << 16)
    } else {
        big = make([]int, 1 << 36)
    }

If we restrict the constants to int, that code will fail at compile
time, because the 1 << 36 will be rejected when building on a 32-bit
system.  Obviously there are various workarounds one can use, such as
build constraints, or using variables rather than constants.  But it
seems reasonable to me to simply permit the simple code, and give the
error at run time if necessary.  It's unlikely that restricting the
constant to int will ever catch any actual problem at compile time.
Permitting an int64 constant will permit some natural code to work
without awkward workarounds.

Ian

[r, 2012-12-07 06:06:27]

I like your example. I remain nervous about the specificity of int64
but it seems likely to be big enough for the time being, so OK.

-rob

[gri, 2012-12-07 06:19:59]

[re-sending, this time to all]

I think the example is broken. That can be written shorter and cleaner as:

var n int
if unsafe.Sizeof(int(1)) == 4 {
    n = 1 << 16
} else {
    n = 1 << 36
}
big := make([]int, n)

or even:

n := 1 << 16
if unsafe.Sizeof(int(1)) == 8 {
    n = 1 << 36
}
big := make([]int, n)

I am starting to lean the other (Rob's) way:

len(a) will return an int and it's simply not possible to create an
array/slice with a larger size. The places where it matters are:

a) array declarations
b) index/slice expressions
c) make()

In a) the size must be a constant and cannot be larger than int (we
certainly don't want a runtime-error for a declaration!). So a)
already requires the size to be within int limits. In fact my current
CL is wrong in this respect.

Arguably, if a) imposes this limit, we should have the same in c)
which is in some sense the dynamic version of c). As shown, Ian's
example can always be trivially rewritten using a variable.

For b) any constant index for an array can impossibly be larger than
int ever, because such arrays cannot be declared. It seems odd to
require a run-time error in that case if the constant is an int.
Again, in the rare case where it might matter, a variable is trivially
solving the problem.

Finally, this only matters when we compile code for a given platform.
I think it makes a lot of sense for that code to not compile if it in
fact either a) declares an array that cannot be represented (size too
large), or make is called with a size too large. Again, if that make
is not dynamically invoked, a variable trivially circumvents the
issue. In some sense this is an issue of writing portable code, not of
the language being restrictive.

I propose:

1) I separate the language change indices -> indexes into a separate
CL (which can be discussed as long as we want)

2) I change the limit to be the maximum int value for the given platform.

- gri

PS: 2) has also the advantage that it almost completely describes the
implemented status quo which in fact has not caused any problems so
far. This issue came up not because of problems with the compilers but
because the compilers implemented something different from what the
spec said.

On Thu, Dec 6, 2012 at 10:06 PM, Rob Pike <r@golang.org> wrote:
> I like your example. I remain nervous about the specificity of int64
> but it seems likely to be big enough for the time being, so OK.
>
> -rob

[iant2, 2012-12-07 06:36:38]

On Thu, Dec 6, 2012 at 10:19 PM, Robert Griesemer <gri@golang.org> wrote:
>
> I think the example is broken. That can be written shorter and cleaner as:
>
> var n int
> if unsafe.Sizeof(int(1)) == 4 {
>     n = 1 << 16
> } else {
>     n = 1 << 36
> }
> big := make([]int, n)

No, you can't write it that way.  Try it with a 32-bit build.  You'll
get something like

foo.go:8: constant 68719476736 overflows int

> or even:
>
> n := 1 << 16
> if unsafe.Sizeof(int(1)) == 8 {
>     n = 1 << 36
> }
> big := make([]int, n)

Doesn't work either, for the same reason.

Ian

[gri, 2012-12-07 06:58:02]

It should be int64 of course. Same argument applies.
On Dec 6, 2012 10:36 PM, "Ian Lance Taylor" <iant@google.com> wrote:

> On Thu, Dec 6, 2012 at 10:19 PM, Robert Griesemer <gri@golang.org> wrote:
> >
> > I think the example is broken. That can be written shorter and cleaner
> as:
> >
> > var n int
> > if unsafe.Sizeof(int(1)) == 4 {
> >     n = 1 << 16
> > } else {
> >     n = 1 << 36
> > }
> > big := make([]int, n)
>
> No, you can't write it that way.  Try it with a 32-bit build.  You'll
> get something like
>
> foo.go:8: constant 68719476736 overflows int
>
> > or even:
> >
> > n := 1 << 16
> > if unsafe.Sizeof(int(1)) == 8 {
> >     n = 1 << 36
> > }
> > big := make([]int, n)
>
> Doesn't work either, for the same reason.
>
> Ian
>

[gri1, 2012-12-07 07:07:53]

So, what I should have written:

var n int64 = 1 << 16
if ...

Otherwise its not portable code of course.

- gri
On Dec 6, 2012 10:58 PM, "Robert Griesemer" <gri@golang.org> wrote:

> It should be int64 of course. Same argument applies.
> On Dec 6, 2012 10:36 PM, "Ian Lance Taylor" <iant@google.com> wrote:
>
>> On Thu, Dec 6, 2012 at 10:19 PM, Robert Griesemer <gri@golang.org> wrote:
>> >
>> > I think the example is broken. That can be written shorter and cleaner
>> as:
>> >
>> > var n int
>> > if unsafe.Sizeof(int(1)) == 4 {
>> >     n = 1 << 16
>> > } else {
>> >     n = 1 << 36
>> > }
>> > big := make([]int, n)
>>
>> No, you can't write it that way.  Try it with a 32-bit build.  You'll
>> get something like
>>
>> foo.go:8: constant 68719476736 overflows int
>>
>> > or even:
>> >
>> > n := 1 << 16
>> > if unsafe.Sizeof(int(1)) == 8 {
>> >     n = 1 << 36
>> > }
>> > big := make([]int, n)
>>
>> Doesn't work either, for the same reason.
>>
>> Ian
>>
>

[rsc, 2012-12-07 07:29:06]

You can write it, you just have to write
    var n int64
(not that most users would figure that out).

Russ

[rsc, 2012-12-11 15:45:39]

This one's dead, right? Removing reviewers.

[gri, 2012-12-12 19:09:18]

*** Abandoned ***