search

mdempsky / unconvert

Remove unnecessary type conversions from Go source
BSD 3-Clause "New" or "Revised" License
377 stars 26 forks source link

Rewrite in terms of lintutil #18

Closed tamird closed 6 years ago

tamird commented 7 years ago

https://godoc.org/honnef.co/go/lint/lintutil is a neat package by @dominikh that provides a framework for writing linters like unconvert.

The main benefit of rewriting unconvert in terms of lintutil is that it would allow users to implement "meta linters" that can run many lints while traversing the AST only once. Such a thing already exists in CockroachDB (https://github.com/cockroachdb/cockroach/blob/master/pkg/cmd/metacheck/main.go).

cc @dominikh

dominikh commented 7 years ago

I would hold off on that at least for now, as the API of lintutil is not stable yet and going to change in the near future.

(And, to be pedantic: the AST will still be traversed lots of times. your meta linter avoids the cost of parsing, type checking and SSA building)

tamird commented 7 years ago

Fair enough, thanks for the correction. FWIW, the meta linter saves about 80% of the time-cost of each linter that is added to it (in my testing against CockroachDB).

On Fri, Nov 18, 2016 at 4:48 PM, Dominik Honnef notifications@github.com wrote:

I would hold off on that at least for now, as the API of lintutil is not stable yet and going to change in the near future.

(And, to be pedantic: the AST will still be traversed lots of times. your meta linter avoids the cost of parsing, type checking and SSA building)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/mdempsky/unconvert/issues/18#issuecomment-261650554, or mute the thread https://github.com/notifications/unsubscribe-auth/ABdsPD74wcHWIrhpQi3eM7IthUB3PCA3ks5q_h0QgaJpZM4K29Ry .

dominikh commented 7 years ago

Frankly, at this point, I'd prefer keeping lintutil internal, as I will experiment with alternative/forked SSA packages and the like, so it won't really be fit for general projects for now.

tamird commented 6 years ago

I've more or less done this in https://github.com/cockroachdb/cockroach/pull/19395.

tamird commented 6 years ago

Update: lintutil is now part of https://godoc.org/honnef.co/go/tools which has a stable release, so it seems like doing this is more palatable now. @mdempsky if you're interested, I'd be happy to submit a PR.

dominikh commented 6 years ago

I'm still advising against it. lintutil is still a moving target - it doesn't have a stable release, merely tagged releases; there can and will be breaking changes between releases (also the whole forked SSA thing and the fact that soon, my tools will no longer use go/loader)

mdempsky commented 6 years ago

It sounds like there's nothing to do here at the moment. I'm going to close for now, but please do reopen once there's a usable framework to rebase unconvert on top of. Thanks!