code review 821046: regexp: use panic/recover to handle errors

regexp: use panic/recover to handle errors

[r, 2010-03-31 20:51:06]

Hello rsc (cc: golang-dev@googlegroups.com),

I'd like you to review this change.

[gri, 2010-03-31 21:05:30]

http://codereview.appspot.com/821046/diff/1/2
File src/pkg/regexp/regexp.go (right):

http://codereview.appspot.com/821046/diff/1/2#newcode585
src/pkg/regexp/regexp.go:585: panic(e)
The e here is the inner e, not the one from recover. If !ok, the inner e is an
empty string (the zero value for Error).

[r, 2010-03-31 21:11:59]

Hello rsc, gri (cc: golang-dev@googlegroups.com),

Please take another look.

[rsc1, 2010-03-31 21:12:26]

LGTM modulo the bug gri pointed out

[rsc1, 2010-03-31 21:13:31]

what you have is fine; the below might or might not be clearer

http://codereview.appspot.com/821046/diff/2002/5001
File src/pkg/regexp/regexp.go (right):

http://codereview.appspot.com/821046/diff/2002/5001#newcode583
src/pkg/regexp/regexp.go:583: if _, ok := e.(Error); !ok {
if e, ok := e.(Error); ok {
    err = e
} else {
    panic(e)
}

[r2, 2010-03-31 21:15:26]

On Mar 31, 2010, at 2:13 PM, rsc@google.com wrote:

> what you have is fine; the below might or might not be clearer
>
>
>
> http://codereview.appspot.com/821046/diff/2002/5001
> File src/pkg/regexp/regexp.go (right):
>
> http://codereview.appspot.com/821046/diff/2002/5001#newcode583
> src/pkg/regexp/regexp.go:583: if _, ok := e.(Error); !ok {
> if e, ok := e.(Error); ok {
>    err = e
> } else {
>    panic(e)
> }

doesn't this have gri's bug again?

>
> http://codereview.appspot.com/821046/show

[rsc, 2010-03-31 21:16:24]

>> if e, ok := e.(Error); ok {
>>   err = e
>> } else {
>>   panic(e)
>> }
>
> doesn't this have gri's bug again?

yes, sigh
when i started writing it i meant

if e, ok := e.(Error); ok {
    err = e
    return
}
panic(e)

[gri, 2010-03-31 21:16:44]

LGTM

On Wed, Mar 31, 2010 at 2:11 PM, <r@golang.org> wrote:

> Hello rsc, gri (cc: golang-dev@googlegroups.com),
>
> Please take another look.
>
>
>
> http://codereview.appspot.com/821046/show
>

[gri, 2010-03-31 21:53:16]

LGTM

[r, 2010-03-31 22:58:36]

*** Submitted as http://code.google.com/p/go/source/detail?r=df1fd6ad4db8 ***

regexp: use panic/recover to handle errors

R=rsc, gri
CC=golang-dev
http://codereview.appspot.com/821046