neugram / ng

scripting language integrated with Go
https://neugram.io
BSD 2-Clause "Simplified" License
917 stars 43 forks source link

ng: handling of unexported Go types #170

Open sbinet opened 6 years ago

sbinet commented 6 years ago

consider the following Go pkg1:

package pkg1

import "io"

type reader struct {
    r io.Reader
}

func (r *reader) Read(data []byte) (int, error) {
    return r.r.Read(data)
}

func NewMyReader(r io.Reader) *reader {
    return &reader{r: r}
}

func NewReader(r io.Reader) io.Reader {
    return &reader{r: r}
}

this will work:

$> ng ./simple-ok.ng
int64(5)
hello

where simple-ok.ng is:

import (
    "bytes"
    "io"

    "neugram.io/ng/testdata/pkg1"
)

r := pkg1.NewReader(bytes.NewReader([]byte("hello")))

buf := new(bytes.Buffer)
io.Copy(buf, r)
print(string(buf.Bytes()))

but this will fail with simple-err.ng:

import (
    "bytes"
    "io"

    "neugram.io/ng/testdata/pkg1"
)

r := pkg1.NewMyReader(bytes.NewReader([]byte("hello")))

buf := new(bytes.Buffer)
io.Copy(buf, r)
print(string(buf.Bytes()))

this boils down to: how to handle (and represent) values of unexported types?

glycerine commented 6 years ago

A simple example I encountered just now, as I tried to set an unexported variable in Sebastien's go-hep, based on ng:

type A struct { newstuff int; newstring string;}
bb := &A{newstuff:55, newstring:"yolo"}
ng: eval: ng eval panic: reflect.StructOf: field "newstuff" is unexported but missing PkgPath
crawshaw commented 6 years ago

The example in @sbinet original comment really should (and I think can be made to) work. But @glycerine points out a trickier fundamental limit of the reflect-based approach.

I think for now I'm going to modify the type checker to reject unexported struct fields in type definitions, so at least the errors are clear.

glycerine commented 6 years ago

I think you can set unexported fields. https://stackoverflow.com/questions/42664837/access-unexported-fields-in-golang-reflect

crawshaw commented 6 years ago

I suppose we already use plenty of unsafe, so we could set unexpected fields. Hmm.

glycerine commented 6 years ago

Encouragement: It would seem odd not to be able to set one's own fields, from within one's own package.

I verified that the stackoverflow technique works. When I replaced

ptrFld := fld.Addr()

with something approximately like

ptrFld := reflect.NewAt(fld.Type(), unsafe.Pointer(fld.UnsafeAddr()))

and then writing to unexported fields went through.

crawshaw commented 6 years ago

This works. Thanks @glycerine. Unfortunately your particular example needs an improvement to reflect.StructOf, which has to wait out the six month Go release cycle: https://golang.org/cl/85661

More generally, we need to come up with rules for setting unexported fields. It is entirely reasonable when sitting in the ng> interpreter to change any field you want. (In fact, there are lots of other "superuser"-style powers I would like to add to the active interpreter.) But in general, an ng program should follow the exported/unexported rules of Go. So loaded packages shouldn't be able to do this. What about writing to unexported fields from inside blocks?

glycerine commented 6 years ago

What about writing to unexported fields from inside blocks?

Cool. I'm not familiar withblocks in this context. Could you give a small example?