Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ng: handling of unexported Go types #170

Open
sbinet opened this issue Dec 19, 2017 · 7 comments
Open

ng: handling of unexported Go types #170

sbinet opened this issue Dec 19, 2017 · 7 comments

Comments

@sbinet
Copy link
Collaborator

sbinet commented Dec 19, 2017

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
Copy link

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
Copy link
Member

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
Copy link

@crawshaw
Copy link
Member

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

@glycerine
Copy link

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 added a commit that referenced this issue Jan 2, 2018
This requires the implementation of reflect.StructOf be relaxed
to allow creating structs with unexported fields.
That is https://golang.org/cl/85661

For #170
@crawshaw
Copy link
Member

crawshaw commented Jan 2, 2018

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
Copy link

What about writing to unexported fields from inside blocks?

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants