[golang-dev] Re: code review 6903048: spec: index and array/slice size constants must fit into int (issue 6903048)

PTAL


https://codereview.appspot.com/6903048/diff/21003/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6903048/diff/21003/doc/go_spec.html#newcode839
doc/go_spec.html:839: array length further, but it must always permit at
least <code>int32</code> values.
This is essentially what I had at first. Reverted.

https://codereview.appspot.com/6903048/diff/21003/doc/go_spec.html#newcode840
doc/go_spec.html:840: For instance, on a 64bit platform, the maximum
length may be restricted by the size
ACK

https://codereview.appspot.com/6903048/

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode840
doc/go_spec.html:840: For instance, the maximum size of an array type
may be restricted by
But now you are back to not setting a lower bound. That was my original
complaint, and rsc mentioned it also.

https://codereview.appspot.com/6903048/

Ian, do you have any suggestions on the lower bound? In general if the
element size is large enough the lower bound might be 1.

I think not setting a lower bound is fine. I added the restriction
such that implementations are free to enforce sensible restrictions
(such as gc does now) and don't have to resort to some kind of runtime
error. If an implementation interprets the absence of a lower bound as
"the maximum permitted size is 10 elements" that's fine too - but
nobody is going to take that compiler serious...

- gri

I still think we should have a lower bound, as we do in our other
implementation restrictions.

"Implementation restriction: an implementation may restrict the maximum
permissible array length. For instance, the maximum size of an array
type may be restricted by the size of the target's virtual address
space. That said, every implementation must support arrays for which
unsafe.Sizeof applied to a value of the array type will return less than
1073741824 (1G)."

https://codereview.appspot.com/6903048/

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode820
doc/go_spec.html:820: of type <code>int</code>.
"representable by a value of type int" is sort of a category error. a
value is a value; it's not representing anything. i think you mean
stored in a variable of type int, but that sounds wrong, and the same
glitch occurs elsewhere. you've added a lot more instances of this, so
let's clean it up everywhere.

since a type represents a range of values (we've used that idea when
introducing types), how about just 'representable by an int'? or maybe
'in the range of the int type'?

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode840
doc/go_spec.html:840: For instance, the maximum size of an array type
may be restricted by
what is this clause trying to achieve, exactly? a compile-time
constraint? a run-time-constraint? mere ass-covering?

https://codereview.appspot.com/6903048/

Ian wants to make sure that implementations can reject
[1<<20][1<<20][1<<20]byte (no current machine can store one of those
in its virtual address space) but that they cannot reject
[5][5][5]byte.

No changes but some feedback.


https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode820
doc/go_spec.html:820: of type <code>int</code>.
I am fine with changing this, but:

a) what you are suggesting is close to what I had at first, and iant
objected;
b) we are using the current phrase "representable by a value of type T"
elsewhere fairly consistently (see Assignability, Conversions, Constant
expressions, etc.).

(The notions of "representable as a value of type int", "integer value",
"value of integer type", and "value of int type" are all very similar in
English, but they all mean different things in the spec. It's probably
worthwhile making the nomenclature clear in the spec at some point but
not in this CL).

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode840
doc/go_spec.html:840: For instance, the maximum size of an array type
may be restricted by
quoting rsc: "I think it would be fine not to set a lower bound and
say..."
(what I have written now).

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode840
doc/go_spec.html:840: For instance, the maximum size of an array type
may be restricted by
It is a compile-time constraint currently implemented by 6g. The clause
is merely permitting an implementation to have such a constraint (rather
than having to resort to some other error - say out of memory, at
run-time). So yes, "ass-covering"...

https://codereview.appspot.com/6903048/

FYI


https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html
File doc/go_spec.html (right):

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode820
doc/go_spec.html:820: of type <code>int</code>.
a value is
variable
elsewhere. you've
range of
I objected mainly because of the inconsistency: I saw that other places
in the spec used different wording to express the concept, and I thought
that we should use that same wording in this CL. If you change the rest
of the spec to use some new wording, I'm OK with that.

I would normally say something like "the constant must be representable
in the type int" but I want the spec to be consistent in the words used
for this concept.

https://codereview.appspot.com/6903048/diff/18002/doc/go_spec.html#newcode840
doc/go_spec.html:840: For instance, the maximum size of an array type
may be restricted by
original

Sorry, you're quite right, I did misread rsc.

But if we permit the implementation to have an upper bound I still want
a lower bound.

https://codereview.appspot.com/6903048/

Specific wording aside, it sounds like everybody is ok with the actual
content of the proposed change (array/slice sizes/indices must fit
into int). If so, I can update/remove some of the issues filed. I'll
take silence as a yes...

Apropos the implementation restriction: perhaps I should just remove it.
- gri