neugram / ng

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

ng: issue with Methodik on non-exported type #96

Open sbinet opened 6 years ago

sbinet commented 6 years ago

not completely sure what is the real underlying issue here... but here it goes:

$> go get go-hep.org/x/hep/rootio
$> cd $GOPATH/src/go-hep.org/x/hep/rootio/testdata
$> ng
ng> import "go-hep.org/x/hep/rootio"
ng> f := rootio.Open("small-flat-tree.root")
ng> t := f.Get("tree")
ng> t.Entries()
ng: typecheck: &{%!s(*expr.Ident=&{t}) %!s(*expr.Ident=&{Entries})} undefined (type format: unknown type: &tipe.Methodik{
    Spec: tipe.Specialization{
        Num: "",
    },
    Type: &tipe.Interface{
        Methods: map[string]*tipe.Func{Class: &tipe.Func{
            Spec: tipe.Specialization{
                Num: "",
            },
            Params: &tipe.Tuple{
                Elems: []tipe.Type{},
            },
            Results: &tipe.Tuple{
                Elems: []tipe.Type{
                    "string",
                },
            },
        }},
    },
    PkgName: "rootio",
    PkgPath: "go-hep.org/x/hep/rootio",
    Name: "Object",
} is not a struct or package)

ng> 
sbinet commented 6 years ago

hum... this may not be an issue with calling a method (through an interface) of an unexported type: the following fails too:

$> cd $GOPATH/src/go-hep.org/x/hep/hbook/rootcnv/testdata
$> ng
ng> import "go-hep.org/x/hep/rootio"
ng> import "fmt"
ng> ff := rootio.Open("gauss-h1.root")
ng> oo := ff.Get("h1d")
ng> _ = fmt.Printf("o=%T\n",oo)
o=*rootio.H1D
ng> oo.Entries()
ng: typecheck: &{%!s(*expr.Ident=&{oo}) %!s(*expr.Ident=&{Entries})} undefined (type format: unknown type: &tipe.Methodik{
    Spec: tipe.Specialization{
        Num: "",
    },
    Type: &tipe.Interface{
        Methods: map[string]*tipe.Func{Class: &tipe.Func{
            Spec: tipe.Specialization{
                Num: "",
            },
            Params: &tipe.Tuple{
                Elems: []tipe.Type{},
            },
            Results: &tipe.Tuple{
                Elems: []tipe.Type{
                    "string",
                },
            },
        }},
    },
    PkgName: "rootio",
    PkgPath: "go-hep.org/x/hep/rootio",
    Name: "Object",
} is not a struct or package)
crawshaw commented 6 years ago

OK, that's a terrible error message. Sorry about that.

Poking around the godoc, it looks like ff.Get should return a rootio.Object, and that's the type of oo. The %T operator in fmt strips off the interface and shows the concrete type underneath it (e.g. https://play.golang.org/p/Fw3QJ-26Al).

I don't have a valid root file to test this with, but I'm guessing this really should be an error. Does oo.(*rootio.H1D).Entries() work?

sbinet commented 6 years ago

no worries :)

that doesn't work either: I fall into issue neugram/ng#97 :)

crawshaw commented 6 years ago

Lots of good bugs coming out of this. It looks like the bug as originally described here holds:

ng> import "go-hep.org/x/hep/rootio"
ng> ff := rootio.Open($$ echo -n $GOPATH/src/go-hep.org/x/hep/hbook/rootcnv/testdata/gauss-h1.root $$)
ng> oo := ff.Get("h1d")
ng> h1d := oo.(*rootio.H1D)
ng> h1d.Entries()
ng: typecheck: &{%!s(*expr.Ident=&{h1d}) %!s(*expr.Ident=&{Entries})} undefined (type *format: unknown type: &tipe.Methodik{
    ... awful error message, but clear that the method set contains Entries ...
} is not a struct or package)
sbinet commented 6 years ago

yeah, I think the problem is in the typechecker, in the *expr.Selector handling, that doesn't handle the *tipe.Methodik case (it only handles *tipe.Package and *tipe.Struct), around line 2180.

crawshaw commented 6 years ago

The method logic is in the loop above that, around L2167. The problem is the Entries method isn't being found in that loop.

crawshaw commented 6 years ago

Like all bugs when working with Go's type system, this is probably going to involve embedding.

sbinet commented 6 years ago

It's true the interfaces I am exposing are embedding other interfaces, and some of the concrete types I am using do too :)

crawshaw commented 6 years ago

Yes, type H1D embeds th1 to get the Entries method. The type computed for neugram's typechecker includes the th1 field, and shows the th1 type has an Entries method, but it's not bubbling up to H1D. So yes, embedding is what's wrong with that particular example.

Now the question is, should the tipe.Memory object know about embedding (and dig down to get the full method set), or should we compute it up for a methodik object? I lean towards the former, but I have to think through some of the implications.