ast.NewPackage errors for built in types

[Steven Hartland]

If the ast.Files passed to ast.NewPackage includes built in types such as int it returns an error e.g.

file1.go:5:6: undeclared name: int

Is there a way to prevent that?

Playground example: https://play.golang.org/p/Yg30TTzoLHP

My goal is to take multiple files, resolve inter file dependencies e.g. a type referencing another type in a different file and process the resulting ast.Files. So if there is a better way to achieve this I'm all ears.

   Regards

   Steve

[David Finkel]

On Mon, Oct 11, 2021 at 5:48 AM Steven Hartland <ste...@multiplay.co.uk> wrote:

If the ast.Files passed to ast.NewPackage includes built in types such as int it returns an error e.g.

file1.go:5:6: undeclared name: int

Is there a way to prevent that?

Generally, I always add the `builtin` package to the list of packages I'm parsing.

I wrote a little library for exactly this kind of package loading a few years ago:

https://gitlab.com/dfinkel/goastpkg/-/blob/master/go_ast_parser.go

(https://pkg.go.dev/golang.spin-2.net/astpkg)

Playground example: https://play.golang.org/p/Yg30TTzoLHP

My goal is to take multiple files, resolve inter file dependencies e.g. a type referencing another type in a different file and process the resulting ast.Files. So if there is a better way to achieve this I'm all ears.

In general, I've stopped using the `go/ast` internal references as much and have started using resolved `go/types` references as they're more reliable and better-specified.
(golang.org/x/tools/go/packages has a LoadMode flag for generating `go/types.Info` (NeedTypesInfo))

   Regards

   Steve

--
You received this message because you are subscribed to the Google Groups "golang-nuts" group.
To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAHEMsqbJoJxuo3c-mofMtzXXJhYCzV2skW2ZB3ZPY6WtA8%2BxHw%40mail.gmail.com.

[Steven Hartland]

Thanks David, much appreciated, I will have a look at both.

When migrating from go/ast to go/types did you hit anything of note I should look out for?

[K. Alex Mills]

It's been several months since I used these packages but IIRC, go/ast only requires code to parse correctly, while go/types actually runs the type-checker.

Running the type checker implies having a complete picture of all the types used in a Go program, which, in my recollection, usually means reaching out to download third-party dependencies if they aren't already available.

That can be a bit much in my experience, but there's no way around it because of how Go's type system works.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAHEMsqavYgUqPOBo6zB5SsizVB39JO9vNUePnecDN-664e0NUQ%40mail.gmail.com.

[Steven Hartland]

I converted my code to x/tools/go/packages and while it did solve the problem it's VERY slow in comparison.

I have a set of 21 tests operating on a single package which has at most two very basic types, no imports and using go/parser they take 0.011s but with go/packages that increases to 3.548s a 300x slow down.

I'm setting a basic mode: packages.NeedName | packages.NeedSyntax

The package.Load call takes ~220ms whereas ast.NewPackage only takes 2.7µs.

As the resulting ast.File's are pretty much the same, I'm wondering if for my use case packages.Load is doing way more than I need?

Another downside is for tests run in a temporary directory outside of the package space package.Load fails with:

directory /tmp/tests76985775 outside available modules

I fixed it by calling ioutil.TempDir with "." but that's not ideal.

Thoughts?

[K. Alex Mills]

FWIW: I'm not surprised that it's slower because it's handling significantly more of the compilation. The parser is very fast. Adding in type-checking is going to be less fast.

IIRC, the packages package also has to make network calls and do filesystem I/O behind the scenes in some cases. The parser doesn't have to do anything more than parse the code and output the AST.

It could certainly be the case that the packages library is more than you need. Without understanding more about your use-case it will be hard to say for sure.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAHEMsqYMSBUfuOUvptv6UrvBFTwFxjOhJZ5sMN-omOx5ESL5hw%40mail.gmail.com

[Eli Bendersky]

On Fri, Oct 15, 2021 at 2:13 PM Steven Hartland <ste...@multiplay.co.uk> wrote:

I converted my code to x/tools/go/packages and while it did solve the problem it's VERY slow in comparison.

I have a set of 21 tests operating on a single package which has at most two very basic types, no imports and using go/parser they take 0.011s but with go/packages that increases to 3.548s a 300x slow down.

I'm setting a basic mode: packages.NeedName | packages.NeedSyntax

The package.Load call takes ~220ms whereas ast.NewPackage only takes 2.7µs.

Could you post a reproducer of your target package and analysis somewhere? 220ms for packages.Load sounds like a lot. It's true that packages does a lot more work than just the parser (*), but it's not supposed to be that slow. In my tests a simple Load with more functionality takes 60-70ms

(*) The type checking takes a bit of time over just parsing to AST, but the biggest difference is loading multiple files from imports. For type checking you need to know, when you see:

import foo

x := foo.Foo()

What the type of `x` is, so go/packages has to analyze the `foo` package as well.

 

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAHEMsqYMSBUfuOUvptv6UrvBFTwFxjOhJZ5sMN-omOx5ESL5hw%40mail.gmail.com.

[Richard Oudkerk]

You could try building the universe scope for ast.NewPackage from types.Universe.  For example

https://play.golang.org/p/1E5Iu4vW3g9

func NewPackage(fset *token.FileSet, files map[string]*ast.File) (*ast.Package, error) {

univ, err := universe()

if err != nil {

return nil, err

}

return ast.NewPackage(fset, files, dummyImporter, univ)

}

func dummyImporter(imports map[string]*ast.Object, importPath string) (*ast.Object, error) {

pkg := imports[importPath]

if pkg == nil {

pkg = ast.NewObj(ast.Pkg, path.Base(importPath))

pkg.Data = ast.NewScope(nil)

imports[importPath] = pkg

}

return pkg, nil

}

func universe() (*ast.Scope, error) {

u := ast.NewScope(nil)

for _, name := range types.Universe.Names() {

o := types.Universe.Lookup(name)

if o == nil {

return nil, fmt.Errorf("failed to lookup %s in universe scope", name)

}

var objKind ast.ObjKind

switch o.(type) {

case *types.Const, *types.Nil:

objKind = ast.Con

case *types.TypeName:

objKind = ast.Typ

case *types.Builtin:

objKind = ast.Fun

default:

return nil, fmt.Errorf("unexpected builtin %s of type %T", o.Name(), o)

}

obj := ast.NewObj(objKind, name)

if u.Insert(obj) != nil {

return nil, fmt.Errorf("types internal error: double declaration")

}

obj.Decl = u

}

return u, nil

}

[Steven Hartland]

This is an example of code resulting in ~200ms.

This was measured on a laptop with i7-7700HQ under WSL2, so that could be a contributing factor.

package mypkg

type MyType struct {
        String string
}

[Steven Hartland]

Thanks Richard, that allowed me to replace a hand rolled universe scope 👍

My importer varies from yours in that for correct lookups for versioned packages or those with '-' in I had to copy ImportPathToAssumedName from x/tools/internal/imports/fix.go.

func simpleImporter(imports map[string]*ast.Object, path string) (*ast.Object, error) {
        pkg := imports[path]
        if pkg == nil {
                pkg = ast.NewObj(ast.Pkg, ImportPathToAssumedName(path))
                pkg.Data = ast.NewScope(nil) // required by ast.NewPackage for dot-import
                imports[path] = pkg
        }
        return pkg, nil
}

This now works for all cases which don't import external packages. So now I just need to do the on demand load of packages, which I suspect will lead me right back to packages.Load.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/d570a7ce-a780-46d8-a323-f9c26a6c2561n%40googlegroups.com.

[Richard Oudkerk]

I am not sure what "import external packages" means.

Apart dot imports (which I have never seen used for real) why would you need to load the imported packages?

[Eli Bendersky]

On Sat, Oct 16, 2021 at 11:58 AM Steven Hartland <ste...@multiplay.co.uk> wrote:

This is an example of code resulting in ~200ms.

This was measured on a laptop with i7-7700HQ under WSL2, so that could be a contributing factor.

package mypkg

type MyType struct {
        String string
}

On my machine (oldish i7-4771 running Ubuntu 20.04) a simple load (with pretty much the tutorial use of x/tools/go/packages) of this code takes 12-18 ms per run.

x/tools/go/packages (XTGP) does a bunch of work - mostly around calling `go list -json` go discover the files containing code in the module, to parse and typecheck. It also runs `go env`, and opens and reads a bunch of files. You can do this work on your own like the other answer demonstrates, but this will become tricky with external dependencies.

Not sure how you're getting to 200 ms on this; it's possible that in your environment (WSL2 or what not) invoking subprocesses and stat-ing the filesystem is particularly costly

[Steven Hartland]

I need to be able to tell the types of fields, in particular are fields of a struct a native type or a struct themselves.

The ast parse even with a simple importer don’t provide that info. 

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/6aaa7c3a-7ef5-47ea-9f29-75443a4599b6n%40googlegroups.com.

[K. Alex Mills]

I'm guessing here, but you probably do need the full type-checker for that, depending on what you mean by "a native type".

In go we can alias types in two ways.

type A uint8

type B = uint8

There are subtle differences between the two which I don't claim to understand... In any case, AFAIK both are essentially built-in types at runtime.

Suppose these types are declared in a third-party package on GitHub and another package uses A and B (since they're exported) to declare a struct like this.

import "github.com/foo/bar"

type MyStruct struct {

  Byte1 bar.A

  Byte2 bar.B

}

AFAIK, the compiler cannot tell whether Byte1 and Byte2 are builtin types or a struct without seeing their definition hosted on GitHub. Which means downloading, parsing, and type-checking the code from GitHub.

But it gets a little worse. Type-checking the remote code entails downloading any of its dependencies recursively and doing the same thing... I'm sure you can see how this can take some time -- especially for large projects.

Anyway, it sounds to me like what you're asking to do requires the use of the type-checker and because of the dependency management involved I believe the most convenient route to type-checking (at this time) lives in the packages package.

Happy to learn if anyone else knows otherwise.

To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/CAHEMsqaQ9%3Dia2MXrVEXt--Qk3Nrx1UXK2JYU0D2BxBcZX%2B5mxw%40mail.gmail

[Steven Hartland]

Thanks for that. I spent some time over the weekend refactoring from raw ast parser with a simpleImporter and universe to using types.

So far this has provided everything I need, and also massively simplified the code as no need to manually parse the ast.File apart from a basic ast.Visitor to create a lookup map for type comments and position information.

I did find the types docs less consumable than the ast docs as the relationship between the various types and the data in types.Info wasn't particularly clear to me even after reading the tutorial. This resulted in it taking quite a bit of trial and error, which I didn't need with raw ast. However once I combined an ast.Visitor and a pass over info.Defs to create a type name map I had most of all the elements.

Getting the type name from types.Info map had me scratching my head for some time as I couldn't believe that the Types field wouldn't have an easy way to get to the name of the type.

Using types Config.Check didn't have any performance penalties that I saw with packages.Load so I'm hopeful that this is the correct route.

I'd love to hear if there is a neat way to get from info.Types to the name of each type without having to process info.Defs first, but that's more to know if missed something obvious or not 😋

Thanks again to everyone who has provided pointers in this thread, as it demonstrates what great community this is!