code review 7300078: exp/ssa/interp: fix build for Plan 9

exp/ssa/interp: fix build for Plan 9

[ality, 2013-02-10 03:54:33]

Hello adonovan@google.com, minux.ma@gmail.com (cc: golang-dev@googlegroups.com,
lucio.dere@gmail.com),

I'd like you to review this change to
https://code.google.com/p/go/

[lucio, 2013-02-10 04:19:25]

Thank, Anthony, please forgive the belated - now redundant - comment from me.

Lucio.

On 2/10/13, ality@pbrane.org <ality@pbrane.org> wrote:
> Reviewers: adonovan, minux,
>
> Message:
> Hello adonovan@google.com, minux.ma@gmail.com (cc:
> golang-dev@googlegroups.com, lucio.dere@gmail.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> exp/ssa/interp: fix build for Plan 9
>
> Please review this at https://codereview.appspot.com/7300078/
>
> Affected files:
>    M src/pkg/exp/ssa/interp/external_plan9.go
>    M src/pkg/exp/ssa/interp/external_unix.go
>    M src/pkg/exp/ssa/interp/external_windows.go
>
>
> Index: src/pkg/exp/ssa/interp/external_plan9.go
> ===================================================================
> copy from src/pkg/exp/ssa/interp/external_unix.go
> copy to src/pkg/exp/ssa/interp/external_plan9.go
> --- a/src/pkg/exp/ssa/interp/external_unix.go
> +++ b/src/pkg/exp/ssa/interp/external_plan9.go
> @@ -2,8 +2,6 @@
>   // Use of this source code is governed by a BSD-style
>   // license that can be found in the LICENSE file.
>
> -// +build !windows,!plan9
> -
>   package interp
>
>   import (
> @@ -12,10 +10,7 @@
>   )
>
>   func ext۰syscall۰Kill(fn *ssa.Function, args []value, slots []value) value
>
> {
> -	// We could emulate syscall.Syscall but it's more effort.
> -	err := syscall.Kill(args[0].(int), syscall.Signal(args[1].(int)))
> -	err = err // TODO(adonovan): fix: adapt concrete err to interpreted iface
>
> (e.g. call interpreted errors.New)
> -	return iface{}
> +	panic("syscall.Kill not yet implemented")
>   }
>
>   func ext۰syscall۰Write(fn *ssa.Function, args []value, slots []value)
> value {
> @@ -28,5 +23,4 @@
>   	n, _ := syscall.Write(args[0].(int), b)
>   	err := iface{} // TODO(adonovan): fix: adapt concrete err to interpreted
>
> iface.
>   	return tuple{n, err}
> -
>   }
> Index: src/pkg/exp/ssa/interp/external_unix.go
> ===================================================================
> --- a/src/pkg/exp/ssa/interp/external_unix.go
> +++ b/src/pkg/exp/ssa/interp/external_unix.go
> @@ -28,5 +28,4 @@
>   	n, _ := syscall.Write(args[0].(int), b)
>   	err := iface{} // TODO(adonovan): fix: adapt concrete err to interpreted
>
> iface.
>   	return tuple{n, err}
> -
>   }
> Index: src/pkg/exp/ssa/interp/external_windows.go
> ===================================================================
> --- a/src/pkg/exp/ssa/interp/external_windows.go
> +++ b/src/pkg/exp/ssa/interp/external_windows.go
> @@ -2,12 +2,13 @@
>   // Use of this source code is governed by a BSD-style
>   // license that can be found in the LICENSE file.
>
> -// +build windows plan9
> +// +build windows
>
>   package interp
>
>   import (
>   	"exp/ssa"
> +	"syscall"
>   )
>
>   func ext۰syscall۰Kill(fn *ssa.Function, args []value, slots []value) value
>
> {
> @@ -15,5 +16,13 @@
>   }
>
>   func ext۰syscall۰Write(fn *ssa.Function, args []value, slots []value)
> value {
> -	panic("syscall.Write not yet implemented")
> +	// We could emulate syscall.Syscall but it's more effort.
> +	p := args[1].([]value)
> +	b := make([]byte, 0, len(p))
> +	for i := range p {
> +		b = append(b, p[i].(byte))
> +	}
> +	n, _ := syscall.Write(args[0].(syscall.Handle), b)
> +	err := iface{} // TODO(adonovan): fix: adapt concrete err to interpreted
>
> iface.
> +	return tuple{n, err}
>   }
>
>
>

[adonovan, 2013-02-10 05:23:40]

https://codereview.appspot.com/7300078/diff/4004/src/pkg/exp/ssa/interp/exter...
File src/pkg/exp/ssa/interp/external_windows.go (right):

https://codereview.appspot.com/7300078/diff/4004/src/pkg/exp/ssa/interp/exter...
src/pkg/exp/ssa/interp/external_windows.go:25: n, _ :=
syscall.Write(args[0].(syscall.Handle), b)
The set of dynamic types that can appear inside a 'value' interface is finite
and does not include syscall.Handle; perhaps uintptr will work though.  You'd
need to build exp/ssa/ssadump.go and run it on a 'hello world' program to test
it:

% ./ssadump -run hello.go

[minux1, 2013-02-10 21:28:51]

On Sun, Feb 10, 2013 at 2:15 PM, Anthony Martin <ality@pbrane.org> wrote:

> Can anyone test this on a Windows machine?
>
./make.bash succeeded, but building ssadump.go results in linker error:
see https://code.google.com/p/go/issues/detail?id=4785

the code for windows won't handle writes to console right, but i think
this is acceptable.

after issue 4785 is fixed, i can test further on windows.

[brainman, 2013-02-11 01:16:46]

> On Sun, Feb 10, 2013 at 2:15 PM, Anthony Martin <mailto:ality@pbrane.org>
wrote:
> 
> > Can anyone test this on a Windows machine?
> >

Your changes build on Windows fine.

Alex

[akumar_mail.nanosouffle.net, 2013-02-20 06:55:04]

LGTM.

On 11 February 2013 01:16,  <alex.brainman@gmail.com> wrote:
>> On Sun, Feb 10, 2013 at 2:15 PM, Anthony Martin
>
> <mailto:ality@pbrane.org> wrote:
>
>> > Can anyone test this on a Windows machine?
>> >
>
>
> Your changes build on Windows fine.
>
> Alex
>
> https://codereview.appspot.com/7300078/
>
> --
>
> ---You received this message because you are subscribed to the Google Groups
> "golang-dev" group.
>
> To unsubscribe from this group and stop receiving emails from it, send an
> email to golang-dev+unsubscribe@googlegroups.com.
> For more options, visit https://groups.google.com/groups/opt_out.
>
>

[akumar_mail.nanosouffle.net, 2013-02-21 18:51:21]

Where do we stand on this?

On 19 February 2013 22:54, Akshat Kumar <akumar@mail.nanosouffle.net> wrote:
> LGTM.
>
> On 11 February 2013 01:16,  <alex.brainman@gmail.com> wrote:
>>> On Sun, Feb 10, 2013 at 2:15 PM, Anthony Martin
>>
>> <mailto:ality@pbrane.org> wrote:
>>
>>> > Can anyone test this on a Windows machine?
>>> >
>>
>>
>> Your changes build on Windows fine.
>>
>> Alex
>>
>> https://codereview.appspot.com/7300078/
>>
>> --
>>
>> ---You received this message because you are subscribed to the Google Groups
>> "golang-dev" group.
>>
>> To unsubscribe from this group and stop receiving emails from it, send an
>> email to golang-dev+unsubscribe@googlegroups.com.
>> For more options, visit https://groups.google.com/groups/opt_out.
>>
>>

[adonovan, 2013-02-21 18:56:16]

On 2013/02/21 18:51:21, akumar_mail.nanosouffle.net wrote:
> Where do we stand on this?

If you just want to fix a broken build, then do panic("unimplemented") for the
implementations of the relevant external functions.  You can make them do real
work as a follow-up at your convenience.  (I don't know the plan9 syscall
interface so I can't help.)

[rminnich1, 2013-02-21 19:27:53]

hi, can one of the people involved just get this in? If not, I guess I can do it
tonight. This is one of our stuck-at points just now. Thanks!

[ality, 2013-02-22 04:06:33]

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

exp/ssa/interp: fix build for Plan 9

R=adonovan, minux.ma, alex.brainman, akumar, rminnich
CC=golang-dev, lucio.dere
https://codereview.appspot.com/7300078