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

spec: use (*[4]int)(x) to convert slice x into array pointer #395

Closed
rogpeppe opened this issue Dec 8, 2009 · 111 comments
Closed

spec: use (*[4]int)(x) to convert slice x into array pointer #395

rogpeppe opened this issue Dec 8, 2009 · 111 comments

Comments

@rogpeppe
Copy link
Contributor

@rogpeppe rogpeppe commented Dec 8, 2009

Currently, it's possible to convert from an array or array pointer to a slice, but
there's no way of reversing 
this.

A possible syntax could be similar to the current notation for type assertions:

ArrayAssertion  = "." "[" Expression ":" Expression
"]" .

where the operands either side of the ":" are constant expressions.

One motivation for doing this is that using an array pointer allows the compiler to
range check constant 
indices at compile time.

a function like this:

func foo(a []int) int
{
    return a[0] + a[1] + a[2] + a[3];
}

could be turned into:

func foo(a []int) int
{
    b := a.[0:4];
    return b[0] + b[1] + b[2] + b[3];
}

allowing the compiler to check all the bounds once only and give compile-time errors
about out of range 
indices.
@robpike
Copy link
Contributor

@robpike robpike commented Dec 9, 2009

Comment 1:

Labels changed: added languagechange.

Status changed to Thinking.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 10, 2009

Comment 2:

I think this functionality would be nice.
Personally I would rather not assume
that the compiler can subtract arbitrary
expressions (as in b := a.[0:4]) but instead
say explicitly what type I want:
b := (*[4]int)(a[0:4])
The argument against this is that we hoped
introducing x[:] would let us get rid of the
implicit conversion from *[4]int to slice.
Maybe it still does, but we allow the explicit one.
There are certainly compelling cases (mostly
in low-level things like jpeg or sha1 block
processing) where converting a slice to *[N]int
for some N would eliminate many bounds checks
for cheap.

Owner changed to r...@golang.org.

@rogpeppe
Copy link
Contributor Author

@rogpeppe rogpeppe commented Dec 10, 2009

Comment 3:

> b := (*[4]int)(a[0:4])
i thought about this. the problem is that it looks like other type conversions, and
currently no 
type conversion can fail at runtime.
actually, i can't see any reason why we couldn't just use the normal type coercion
syntax:
b := a[0:4]).(*[4]int)
@rsc
Copy link
Contributor

@rsc rsc commented Dec 10, 2009

Comment 4:

Yes, that's a disadvantage of (*[4]int)(a[0:4]).
A disadvantage of a[0:4].(*[4]int) is that it takes over
syntax currently reserved for interface values.  At one
point conversion syntax and type guard syntax was 
interchangeable.  It clarified things quite a bit
to require that in x.(T), x must be an interface value
and that in T(x), the conversion must be statically
guaranteed to succeed.
Unfortunately, this particular conversion doesn't fit 
into either of those categories.
We've got enough going on that's it going to be a while
before we do anything with this.
@rogpeppe
Copy link
Contributor Author

@rogpeppe rogpeppe commented May 11, 2011

Comment 5:

I just encountered a nice example of when this functionality might be useful. To quote
from a reddit user on why Go "needs" pointer arithmetic: 
"One well-used example is making classes as small as possible for tree nodes or linked
list nodes so you can cram as many of them into L1 cache lines as possible. This is done
by each node having a single pointer to a left sub-node, and the right sub-node being
accessed by the pointer to the left sub-node + 1. This saves the 8-bytes for the
right-node pointer. To do this you have to pre-allocate all the nodes in a vector or
array so they're laid out in memory sequentially, but it's worth it when you need it for
performance. (This also has the added benefit of the prefetchers being able to help
things along performance-wise - at least in the linked list case).''
You can *almost* do this in Go with 
   type node struct {
      value int
      children *[2]node
   }
except that there's no way of getting a *[2]node from the underlying slice.
@gopherbot
Copy link

@gopherbot gopherbot commented May 17, 2011

Comment 6 by nmessenger:

If neither syntax is ideal, perhaps a new unslice builtin?
    ary, ok := unslice([n]T, slc)
Though should ary have type [n]T or *[n]T? If n is large and the unslice fails, a large
zeroed array might not be ideal. Anything wrong with this that I'm not seeing? Well,
besides it being another new builtin.
@gopherbot
Copy link

@gopherbot gopherbot commented May 17, 2011

Comment 7 by robpike:

This is a big deal because ary would not have static type.
@rsc
Copy link
Contributor

@rsc rsc commented May 17, 2011

Comment 8:

If we were going to do it - which is far from even up for debate - I think
the syntax x.(*[10]int) works well (x has type []int).  You can't .(T) a slice
type right now, so it would not be overloading anything, and like an
interface type assertion it can fail or be checked at run time.
You can even think of it as []int containing a *[10]int the same way
an interface value holds a concrete type, and you're extracting the
underlying array pointer.
That said, I don't think this is important enough to worry about now.
There is enough else going on.
@rsc
Copy link
Contributor

@rsc rsc commented Dec 9, 2011

Comment 9:

Labels changed: added priority-later.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Jan 29, 2013

Comment 11:

Since this is something that's been bugging me lately too, I thought I'd add a few
random thoughts I had that don't seem to have been mentioned:
Allowing conversions from slices to array-pointers means pointers can now refer to
partially overlapping objects.  I don't believe that's currently possible in the
language.  Slices can already overlap though, so it's not a big change overall.
Like #8 says, []T is sort-of an interface type for *[N]T, so type assertions are
arguably suitable syntax.  Except that cap(x.(*[N]T)) might give a different value than
cap(x), which isn't true for other interfaces/implementor-type relations.  Seems like an
open question whether this inconsistency is worth accepting into the language, and
really since there's already a way to convert a *[N]T into a []T, just the ability to
turn a []T into a *[N]T is the relevant missing feature.
It would be nice if an expression like x[e1:e2] could actually have a static type of
*[e2-e1]T (assuming e1 and e2 are constant expressions), then you could write something
like *dst[16:24] = *src[136:144] and the compiler can verify that the array bounds match
up.  Unfortunately, the expression can't actually be x[e1:e2] since existing code might
rely on cap(x[e1:e2]) == cap(x)-e1, and that would be a backwards incompatible change. 
The x.[e1:e2] syntax suggested originally would solve this issue.
If you want a range like x.[n:n+4], instead of requiring the language to recognize this
pattern somehow, x[n:].[:4] is equivalent and has static indices at the expense of
clunkier notation.  A short-hand notation like x.[n:+4] might be nice to indicate that 4
is a length not an end position, but not strictly necessary and complicates the
language.  (Also +4 here is technically ambiguous here whether it's length 4 or end
position "+4", so again some new notation would be necessary.)
@gopherbot
Copy link

@gopherbot gopherbot commented Jun 8, 2013

Comment 12 by peter.waller:

I just want to note that we have a use case for this at go-gl. More information:
go-gl/gl#111
The underlying OpenGL API only accepts the equivalent of, e.g, a *[4]float32, so it is
nice to have this in the type system on our side. OTOH, a consumer of this API might be
holding a []float32 they want to pass to us. So it would be great to find a solution to
this, as the current solutions are a bit messy or require the use of unsafe.
@rsc
Copy link
Contributor

@rsc rsc commented Nov 27, 2013

Comment 13:

Labels changed: added go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 14:

Labels changed: added release-none, removed go1.3maybe.

@rsc
Copy link
Contributor

@rsc rsc commented Dec 4, 2013

Comment 15:

Labels changed: added repo-main.

@ncw
Copy link
Contributor

@ncw ncw commented Jan 20, 2014

Comment 16:

An alternative which occurred to me was in a function which only indexes a slice with
constants values (eg the JPEG routines or unrolled FFTs which are what I'm working on)
and the slice doesn't change the compiler could bounds check the slice just once at the
start of the function with min(constants) and max(constants).
This would achieve the removal of the bounds checking without a language change.  It
wouldn't allow the compiler to do range checking at compile time though.
@gopherbot
Copy link

@gopherbot gopherbot commented Mar 19, 2014

Comment 17 by matthieu.riou:

Another use case is to be able to use a small slice of known length as a map key. The Go
API uses slices heavily even though the same length is expected. Being able to get the
underlying array would allow usage as map keys.
Two examples I've run into recently are hashes (SHA-256) and IP addresses (as 16 byte
slices). It seems rather wasteful to have to copy them or transform them to strings to
have to use them as map keys.
@nerdatmath
Copy link
Contributor

@nerdatmath nerdatmath commented Apr 11, 2015

FWIW, something similar to unslice() above can be implemented with reflect and unsafe. Despite being implemented in terms of unsafe, I believe unslice itself is safe. I don't know whether it violates any assumptions made by the GC, however.

http://play.golang.org/p/DixtgwxXUH

@daviddengcn
Copy link

@daviddengcn daviddengcn commented Apr 11, 2015

Actually I believe the compiler could easily be smart enough to make a single range check for statement like this:

return a[0] + a[1] + a[2] + a[3]
@chalonga
Copy link

@chalonga chalonga commented Apr 14, 2015

Is there a good way to do this currently?

For example if one function gives me a slice as output and I need to use that output in another function that wants a fixed array as input. What is the best way to coerce the slice into an array that is the current size of the slice and containing it's current members?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Apr 14, 2015

Currently there is no safe way to convert from a slice type to an array type (that is the point of this issue).

You can do it using unsafe by writing code like

(*[10]byte)(unsafe.Pointer(&b[0]))
@josharian
Copy link
Contributor

@josharian josharian commented Mar 12, 2021

I've been slowly updating CL 216424 to implement this proposal as accepted (stay tuned). Two things I've noticed so far, neither dealbreakers, but both a bit unfortunate:

(1)

We have to massage the semantics of reflect.Value.Convert a bit. Current docs:

Convert returns the value v converted to type t. If the usual Go conversion rules do not allow conversion of the value v to type t, Convert panics.

Suggested new docs:

Convert returns the value v converted to type t. If the usual Go conversion rules do not allow conversion of the value v to type t, or if converting v to type t would panic, Convert panics.

Same for reflect.Type.ConvertibleTo:

Currrent docs:

ConvertibleTo reports whether a value of the type is convertible to type u.

Suggested docs:

ConvertibleTo reports whether a value of the type is convertible to type u. Even if ConvertibleTo returns true, the conversion may still panic.

This latter addition may invalidate a common assumption made in code. Because ConvertibleTo has only types to work with, not concrete values, I don't think w e can do any better. We might want to add another Value method like CanConvert that reports whether the conversion will succeed (not panic).

(2)

It is now possible to write a top level var decl that panics:

var (
  s = make([]byte,  5)
  p = (*[10]byte)(s) // panics
)

There's no place to put a recover that'll catch that panic. In normal life, that's not a big deal, but it's an annoyance for writing compiler tests, because there's no in-process way to test that a panic occurs without fundamentally changing the program.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 12, 2021

The reflect.Value.Convert questions are interesting. Changing Convert seems fine. I am a little more nervous about changing ConvertibleTo, but changing it to report whether a conversion would be allowed by the type system seems sensible and consistent. Comparable returns true for interface types, even though comparing an interface value can panic if the interface contains a value whose dynamic type is not comparable.

It is now possible to write a top level var decl that panics:

I don't think this is a new possibility? There are plenty of ways to write var declarations at package scope that panic. E.g.,

var zero int
var _ = 1 / zero

var _ = *(*int)(nil)
@gopherbot
Copy link

@gopherbot gopherbot commented Mar 14, 2021

Change https://golang.org/cl/301651 mentions this issue: go/types: allow conversion from slice to array ptr

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 14, 2021

Change https://golang.org/cl/301650 mentions this issue: cmd/compile: allow conversion from slice to array ptr

@gopherbot
Copy link

@gopherbot gopherbot commented Mar 14, 2021

Change https://golang.org/cl/301652 mentions this issue: reflect: allow conversion from slice to array ptr

@josharian
Copy link
Contributor

@josharian josharian commented Mar 14, 2021

Weird. GopherBot missed a CL: https://golang.org/cl/216424, for the spec. (And that's all--a stack of four.)

@yaxinlx
Copy link

@yaxinlx yaxinlx commented Mar 15, 2021

Does the conversion Ian mentioned in #395 (comment) work according the final design?

type A [4]int
var s = (*A)[]int{1, 2, 3, 4}

If the answer is not, a.k.a. the conversion must be written as (*[4]int)[]int{1, 2, 3, 4}, then it would be some verbose to write the conversion, as the [4]int part in (*[4]int) is the only choice so it is not very essential. It would be even more verbose if the the array type is an type from another package and the package needn't to be imported if the conversion is not used.

@josharian
Copy link
Contributor

@josharian josharian commented Mar 15, 2021

@yaxinlx it does. Although you are missing some parens in your code:

type A [4]int
var s = (*A)([]int{1, 2, 3, 4})

Feel free to try it out using the CLs above.

@josharian
Copy link
Contributor

@josharian josharian commented Mar 15, 2021

In the spec CL @randall77 asks whether we should compare the array length to the slice capacity or the slice length. The discussion so far as been rather ambiguous on this question. We should resolve it here.

@randall77 says:

Using capacity is safe. But I often envision the space between length and cap to be "uninitialized" storage. This lets you access those elements without reslicing first.

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Mar 15, 2021

I have always assumed that the length of the slice must equal the length of the pointed-to array. I don't think capacity figures in here at all. After all, converting []byte to a string converts up to the length of the slice, not the capacity of the slice.

@bcmills
Copy link
Member

@bcmills bcmills commented Mar 15, 2021

Personally I'd prefer that we use the length of the slice.
If a[i] would panic for the original slice, then I think (*A)(a)[i] should either panic or be statically disallowed.

@josharian
Copy link
Contributor

@josharian josharian commented Mar 15, 2021

the length of the slice must equal the length of the pointed-to array

Why == instead of >=? The conversation above has pretty consistently discussed >=.

@mdempsky
Copy link
Member

@mdempsky mdempsky commented Mar 15, 2021

To simplify measuring opinions here, I recommend:

  • On Bryan's last comment (2 above this one), vote:

    • 👍 if the conversion should be allowed based on the slice's length
    • 👎 if the conversion should be allowed based on the slice's capacity
  • On Josh's last comment (1 above this one), vote:

    • 👍 if the conversion test should be compared using >=
    • 👎 if the conversion test should be compared using ==
@josharian
Copy link
Contributor

@josharian josharian commented Mar 15, 2021

Seems like there are some clear trends. :) I'll update the CLs.

@nightlyone
Copy link
Contributor

@nightlyone nightlyone commented Mar 17, 2021

@josharian I didn't see any release notes related to this. Could you make sure any of the changes get a RELNOTES=yes Tag so it won't be forgotten?

@josharian
Copy link
Contributor

@josharian josharian commented Mar 17, 2021

@nightlyone done. (None of them have been submitted yet, waiting on reviews.)

@betawaffle
Copy link

@betawaffle betawaffle commented Apr 13, 2021

What about allowing conversion from *T to *[1]T as well? I would find that quite useful when trying to call a function that accepts a slice, but I have a single item already allocated from elsewhere.

@bcmills
Copy link
Member

@bcmills bcmills commented Apr 13, 2021

@betawaffle, that's an interesting idea, but this proposal was already approved as-is — want to file a separate proposal for converting between *T and *[1]T?

@betawaffle
Copy link

@betawaffle betawaffle commented Apr 13, 2021

@bcmills: Thanks for the suggestion. Opened #45545.

gopherbot pushed a commit that referenced this issue Apr 20, 2021
Implementation follows in subsequent changes.

Updates #395

Change-Id: Ic97ee822805e4c236fdd9d224e776cb2ae62c817
Reviewed-on: https://go-review.googlesource.com/c/go/+/216424
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
gopherbot pushed a commit that referenced this issue Apr 21, 2021
Panic if the slice is too short.

Updates #395

Change-Id: I90f4bff2da5d8f3148ba06d2482084f32b25c29a
Reviewed-on: https://go-review.googlesource.com/c/go/+/301650
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
gopherbot pushed a commit that referenced this issue Apr 21, 2021
These match the changes to cmd/compile/internal/types2 in CL 301650.

Updates #395

Change-Id: I1e85b6355c8c8fdba0996c26a2505c65fab908d6
Reviewed-on: https://go-review.googlesource.com/c/go/+/301651
Trust: Josh Bleecher Snyder <josharian@gmail.com>
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Josh Bleecher Snyder <josharian@gmail.com>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Reviewed-by: Robert Griesemer <gri@golang.org>
@gopherbot gopherbot closed this in 760d3b2 Apr 21, 2021
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 21, 2021

Change https://golang.org/cl/312070 mentions this issue: cmd/compile: allow export/import OSLICE2ARRPTR

gopherbot pushed a commit that referenced this issue Apr 21, 2021
Updates #395
Fixes #45665

Change-Id: Iaf053c0439a573e9193d40942fbdb22ac3b4d3bb
Reviewed-on: https://go-review.googlesource.com/c/go/+/312070
Trust: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Run-TryBot: Cuong Manh Le <cuong.manhle.vn@gmail.com>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
TryBot-Result: Go Bot <gobot@golang.org>
@gopherbot
Copy link

@gopherbot gopherbot commented Apr 29, 2021

Change https://golang.org/cl/315169 mentions this issue: cmd/compile/internal/types2: slice-to-array-pointer conversion requires go1.17

gopherbot pushed a commit that referenced this issue Apr 29, 2021
…es go1.17

Add missing version check. Even though this is a new types2 error
we separate between the compiler and the types2 error message: we
have the compiler error message to match the compiler style, and
we have a types2-specific error message to match the types2 style
for these kinds of errors (for now).

Eventually we need to decide which style we like better and clean
this up.

Follow-up on https://golang.org/cl/301650.

Updates #395.

Change-Id: I5b779f345994c66b1f4a4db466466f98b7d3c491
Reviewed-on: https://go-review.googlesource.com/c/go/+/315169
Trust: Robert Griesemer <gri@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Proposals
Accepted
Linked pull requests

Successfully merging a pull request may close this issue.

None yet