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

Jump to bottom

x/tools/go/loader: TypeCheckFuncBodies doesn't work with packages specified as files #18288

Closed
mvdan opened this issue Dec 12, 2016 · 6 comments
Closed

x/tools/go/loader: TypeCheckFuncBodies doesn't work with packages specified as files #18288

mvdan opened this issue Dec 12, 2016 · 6 comments
Labels
FrozenDueToAge
Milestone
Unreleased

Comments

@mvdan
Copy link
Member

mvdan commented Dec 12, 2016 •
edited

go/loader.Config has the following:

// TypeCheckFuncBodies is a predicate over package paths.
// A package for which the predicate is false will
// have its package-level declarations type checked, but not
// its function bodies; this can be used to quickly load
// dependencies from source.  If nil, all func bodies are type
// checked.
TypeCheckFuncBodies func(path string) bool

I wrote a linter that needs to typecheck func bodies for the packages/files it is linting. But outside of those packages, doing that work is wasting time. For example, typechecking all of the std dependencies can bump the run time from ~0.3s to ~1.2s when running the linter on a small package.

I've tried getting around it with something like the following:

paths := gotool.ImportPaths(args)
isArgPkg := make(map[string]bool, len(paths))
for _, path := range paths {
        isArgPkg[path] = true
}
config := loader.Config{TypeCheckFuncBodies: func(path string) bool {
        return isArgPkg[path]
}}
if _, err := config.FromArgs(paths, false); err != nil {
        return err
}
prog, err := config.Load()

To clarify, gotool is a clone of Go's unexported code that handles package arguments. This runs fine if using the linter with arguments like ., ./..., mypackage or bytes.

But it breaks if using filenames like foo.go, because paths will be []string{"foo.go"}. The package node hasn't been read yet. I assume the call that ultimately reads it is config.Load() - but by then, it's too late to set TypeCheckFuncBodies.

Is there a way to do this? If not, I would suggest the API should allow it somehow.

CC @alandonovan
@mvdan
Copy link
Member Author

mvdan commented Dec 12, 2016

Another way to pharse this - I only want to typecheck the packages specified by args, and not their dependencies, to save time.

@mvdan
Copy link
Member Author

mvdan commented Dec 23, 2016

Maybe I meant @adonovan?

@griesemer griesemer added this to the Unreleased milestone Dec 24, 2016
@mvdan mvdan mentioned this issue Mar 20, 2017
Closed
@alandonovan
Copy link
Contributor

I only want to typecheck the packages specified by args, and not their dependencies, to save time.

You can't accurately type-check a package without type information for each package that it imports.

The gc and gccgo compilers save type information from one package and use it when compiling the next one; this information about package P can be found in a section of the archive library $GOPATH/pkg/$GOOS_GOARCH/${P}.a. This works well for a compiler because the build tool (go build) ensures that dependencies are built in order. However, the loader is not part of a build tool and so cannot assume that the .a files in $GOPATH/pkg are up to date, or exist at all. So, it loads source code for all the packages transitively imported by the initial packages. This is somewhat wasteful, but simple to understand and implement and fast enough in practice for most users, though Go workspaces are getting larger and it may be time to consider making the loader stash type information too.

You're right that the TypeCheckFuncBodies hook doesn't work with packages specified as files. That's something to think about as we reconsider the API.

@mvdan
Copy link
Member Author

mvdan commented Mar 27, 2017

Thanks for the update. Should this issue remain open about TypeCheckFuncBodies working with files in the updated API?

@alandonovan
Copy link
Contributor

Yes, it's a valid bug report, so let's keep it open.

@mvdan mvdan changed the title x/tools/go/loader: no way to only type check supplied packages x/tools/go/loader: TypeCheckFuncBodies doesn't work with packages specified as files Jan 18, 2018
@mvdan
Copy link
Member Author

mvdan commented Oct 28, 2018

go/packages with LoadSyntax solves this for me with flying colors - it uses GOCACHE, and even populates it if anything is missing. Loading a small project with tons of dependencies is now fast, once the cache is warm. I don't think there's a reason to keep this open.

@mvdan mvdan closed this as completed Oct 28, 2018
@golang golang locked and limited conversation to collaborators Oct 28, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
FrozenDueToAge
Projects
None yet
Milestone
Unreleased
Development

No branches or pull requests
4 participants
@mvdan @alandonovan @griesemer @gopherbot