code review 130210043: runtime: move panicindex/panicslice to Go.

runtime: move panicindex/panicslice to Go.

[khr, 2014-08-18 20:16:17]

Hello golang-codereviews@googlegroups.com,

I'd like you to review this change to
https://khr%40golang.org@code.google.com/p/go/

[bradfitz, 2014-08-18 20:21:03]

LGTM

You're getting to the hard stuff.



On Mon, Aug 18, 2014 at 1:16 PM, <khr@golang.org> wrote:

> Reviewers: golang-codereviews,
>
> Message:
> Hello golang-codereviews@googlegroups.com,
>
> I'd like you to review this change to
> https://khr%40golang.org@code.google.com/p/go/
>
>
> Description:
> runtime: move panicindex/panicslice to Go.
>
> Please review this at https://codereview.appspot.com/130210043/
>
> Affected files (+13, -12 lines):
>   M src/pkg/runtime/panic.c
>   A src/pkg/runtime/panic.go
>
>
> Index: src/pkg/runtime/panic.c
> ===================================================================
> --- a/src/pkg/runtime/panic.c
> +++ b/src/pkg/runtime/panic.c
> @@ -460,18 +460,6 @@
>  }
>
>  void
> -runtime·panicindex(void)
> -{
> -       runtime·panicstring("index out of range");
> -}
> -
> -void
> -runtime·panicslice(void)
> -{
> -       runtime·panicstring("slice bounds out of range");
> -}
> -
> -void
>  runtime·throwreturn(void)
>  {
>         // can only happen if compiler is broken
> Index: src/pkg/runtime/panic.go
> ===================================================================
> new file mode 100644
> --- /dev/null
> +++ b/src/pkg/runtime/panic.go
> @@ -0,0 +1,13 @@
> +// Copyright 2014 The Go Authors. All rights reserved.
> +// Use of this source code is governed by a BSD-style
> +// license that can be found in the LICENSE file.
> +
> +package runtime
> +
> +func panicindex() {
> +       panic(errorString("index out of range"))
> +}
> +
> +func panicslice() {
> +       panic(errorString("slice bounds out of range"))
> +}
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.
>

[khr, 2014-08-18 20:26:31]

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

runtime: move panicindex/panicslice to Go.

LGTM=bradfitz
R=golang-codereviews, bradfitz
CC=golang-codereviews
https://codereview.appspot.com/130210043

[dave_cheney.net, 2014-08-18 21:33:43]

Is there a chance that one day panicindex might tell you the value of
the index that was out of range ?

On Tue, Aug 19, 2014 at 6:26 AM,  <khr@golang.org> wrote:
> *** Submitted as
> https://code.google.com/p/go/source/detail?r=3cf190969915 ***
>
>
> runtime: move panicindex/panicslice to Go.
>
> LGTM=bradfitz
> R=golang-codereviews, bradfitz
> CC=golang-codereviews
> https://codereview.appspot.com/130210043
>
>
> https://codereview.appspot.com/130210043/
>
>
> --
> You received this message because you are subscribed to the Google Groups
> "golang-codereviews" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-codereviews+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/d/optout.

[khr1, 2014-08-18 21:56:05]

That would be nice.  I believe the objection to it is that it bloats the
executable to pass that info on every failed bounds check.  There are a lot
of them and each would require an additional store (or 2?  You probably
want both the index and the length).

I don't have any actual numbers; it would be interesting to know what the
bloat would be.




On Mon, Aug 18, 2014 at 2:33 PM, Dave Cheney <dave@cheney.net> wrote:

> Is there a chance that one day panicindex might tell you the value of
> the index that was out of range ?
>
> On Tue, Aug 19, 2014 at 6:26 AM,  <khr@golang.org> wrote:
> > *** Submitted as
> > https://code.google.com/p/go/source/detail?r=3cf190969915 ***
> >
> >
> > runtime: move panicindex/panicslice to Go.
> >
> > LGTM=bradfitz
> > R=golang-codereviews, bradfitz
> > CC=golang-codereviews
> > https://codereview.appspot.com/130210043
> >
> >
> > https://codereview.appspot.com/130210043/
> >
> >
> > --
> > You received this message because you are subscribed to the Google Groups
> > "golang-codereviews" group.
> > To unsubscribe from this group and stop receiving emails from it, send an
> > email to golang-codereviews+unsubscribe@googlegroups.com.
> > For more options, visit https://groups.google.com/d/optout.
>

[dave_cheney.net, 2014-08-18 22:12:15]

Yes. I think you are right on all counts.
On 19 Aug 2014 07:56, "Keith Randall" <khr@google.com> wrote:

> That would be nice.  I believe the objection to it is that it bloats the
> executable to pass that info on every failed bounds check.  There are a lot
> of them and each would require an additional store (or 2?  You probably
> want both the index and the length).
>
> I don't have any actual numbers; it would be interesting to know what the
> bloat would be.
>
>
>
>
> On Mon, Aug 18, 2014 at 2:33 PM, Dave Cheney <dave@cheney.net> wrote:
>
>> Is there a chance that one day panicindex might tell you the value of
>> the index that was out of range ?
>>
>> On Tue, Aug 19, 2014 at 6:26 AM,  <khr@golang.org> wrote:
>> > *** Submitted as
>> > https://code.google.com/p/go/source/detail?r=3cf190969915 ***
>> >
>> >
>> > runtime: move panicindex/panicslice to Go.
>> >
>> > LGTM=bradfitz
>> > R=golang-codereviews, bradfitz
>> > CC=golang-codereviews
>> > https://codereview.appspot.com/130210043
>> >
>> >
>> > https://codereview.appspot.com/130210043/
>> >
>> >
>> > --
>> > You received this message because you are subscribed to the Google
>> Groups
>> > "golang-codereviews" group.
>> > To unsubscribe from this group and stop receiving emails from it, send
>> an
>> > email to golang-codereviews+unsubscribe@googlegroups.com.
>> > For more options, visit https://groups.google.com/d/optout.
>>
>
>