Merge lp:~dave-cheney/juju-core/110-fix-lp-1251076 into lp:~go-bot/juju-core/trunk

[Dave Cheney]

Workaround LP 1251076

gccgo appears to get confused and thinks that out[1] is not nil when in fact it is an interface value of type error and value nil.

This workaround solves the problem by leaving error as nil if in fact it was nil and causes no harm for gc because the predicates above assert that if out[1] is not nil, then it is an error type.

https://codereview.appspot.com/83340045/

Workaround LP 1251076

gccgo appears to get confused and thinks that out[1] is not nil when in fact it is an interface value of type error and value nil.

This workaround solves the problem by leaving error as nil if in fact it was nil and causes no harm for gc because the predicates above assert that if out[1] is not nil, then it is an error type.

https://codereview.appspot.com/83340045/

[Dave Cheney (dave-cheney)]

Reviewers: mp+213764_code.launchpad.net,

Message:
Please take a look.

Description:
Workaround LP 1251076

gccgo appears to get confused and things that out[1] is not nil when in
fact it is an interface value of type error and value nil.

This workaround solves the problem by leaving error as nil if in fact it
was nil and causes no harm for gc because the predicates above assert
that if out[1] is not nil, then it is an error type.

https://code.launchpad.net/~dave-cheney/juju-core/110-fix-lp-1251076/+merge/213764

(do not edit description out of merge proposal)

Please review this at https://codereview.appspot.com/83340045/

Affected files (+9, -1 lines):
   A [revision details]
   M rpc/rpcreflect/type.go

Index: [revision details]
=== added file '[revision details]'
--- [revision details] 2012-01-01 00:00:00 +0000
+++ [revision details] 2012-01-01 00:00:00 +0000
@@ -0,0 +1,2 @@
+Old revision: tarmac-20140402024520-p4uc9yiaf00lnjcy
+New revision: <email address hidden>

Index: rpc/rpcreflect/type.go
=== modified file 'rpc/rpcreflect/type.go'
--- rpc/rpcreflect/type.go 2013-09-24 07:12:05 +0000
+++ rpc/rpcreflect/type.go 2014-04-02 04:55:38 +0000
@@ -144,7 +144,13 @@
   f := func(rcvr reflect.Value, id string) (r reflect.Value, err error) {
    out := rcvr.Method(m.Index).Call([]reflect.Value{reflect.ValueOf(id)})
    if !out[1].IsNil() {
- err = out[1].Interface().(error)
+ // Workaround LP 1251076.
+ // gccgo appears to get confused and things that out[1] is not nil when
+ // in fact it is an interface value of type error and value nil.
+ // This workaround solves the problem by leaving error as nil if in fact
+ // it was nil and causes no harm for gc because the predicates above
+ // assert that if out[1] is not nil, then it is an error type.
+ err, _ = out[1].Interface().(error)
    }
    r = out[0]
    return

[Dave Cheney (dave-cheney)]

Please take a look.

https://codereview.appspot.com/83340045/

[William Reade (fwereade)]

LGTM

https://codereview.appspot.com/83340045/

[Dave Cheney (dave-cheney)]

Thanks William. I'd like to hear from Rog as well as I stared at this
one for several hours this afternoon.

On Wed, Apr 2, 2014 at 6:21 PM, William Reade
<email address hidden> wrote:
> LGTM
>
> https://codereview.appspot.com/83340045/
>
> --
> https://code.launchpad.net/~dave-cheney/juju-core/110-fix-lp-1251076/+merge/213764
> You are the owner of lp:~dave-cheney/juju-core/110-fix-lp-1251076.

[Dave Cheney (dave-cheney)]

Roger, last chance to comment before I submit this thing so I can get
the rpc tests passing on ppc64

On Wed, Apr 2, 2014 at 6:33 PM, David Cheney <email address hidden> wrote:
> Thanks William. I'd like to hear from Rog as well as I stared at this
> one for several hours this afternoon.
>
> On Wed, Apr 2, 2014 at 6:21 PM, William Reade
> <email address hidden> wrote:
>> LGTM
>>
>> https://codereview.appspot.com/83340045/
>>
>> --
>> https://code.launchpad.net/~dave-cheney/juju-core/110-fix-lp-1251076/+merge/213764
>> You are the owner of lp:~dave-cheney/juju-core/110-fix-lp-1251076.

=== modified file 'rpc/rpcreflect/type.go'
--- rpc/rpcreflect/type.go	2013-09-24 07:12:05 +0000
+++ rpc/rpcreflect/type.go	2014-04-02 05:00:50 +0000
@@ -144,7 +144,13 @@
 	f := func(rcvr reflect.Value, id string) (r reflect.Value, err error) {
 		out := rcvr.Method(m.Index).Call([]reflect.Value{reflect.ValueOf(id)})
 		if !out[1].IsNil() {
-			err = out[1].Interface().(error)
+			// Workaround LP 1251076.
+			// gccgo appears to get confused and thinks that out[1] is not nil when
+			// in fact it is an interface value of type error and value nil.
+			// This workaround solves the problem by leaving error as nil if in fact
+			// it was nil and causes no harm for gc because the predicates above
+			// assert that if out[1] is not nil, then it is an error type.
+			err, _ = out[1].Interface().(error)
 		}
 		r = out[0]
 		return