[HN Gopher] Deadcode: Finding unreachable functions in Go
___________________________________________________________________
Deadcode: Finding unreachable functions in Go
Author : rbanffy
Score : 97 points
Date : 2024-01-23 18:05 UTC (4 hours ago)
(HTM) web link (go.dev)
(TXT) w3m dump (go.dev)
| jstanley wrote:
| Cool but please don't make it a compile error!
| AnimalMuppet wrote:
| Well, unused imports are a compile error, so there's precedent.
| jstanley wrote:
| Yes, unreachable code after a "return" in a function is also
| a compile error. Very frustrating when I just want to nop out
| part of a function for debugging something else and have to
| keep fighting the compiler until I find a way that is
| acceptable.
| fooster wrote:
| Huh? Its a warning, not an error.
|
| https://go.dev/play/p/8GazBQv5xAb
| jstanley wrote:
| My mistake!
| packetlost wrote:
| If you're finding you're doing this often, perhaps your
| functions are too large
| starttoaster wrote:
| If you find yourself doing this often, maybe a feature flag
| pattern using environment variables would end up being a
| bit nicer looking than throwing a bunch of unreachable code
| into your code base.
| radicalbyte wrote:
| It would break reflection surely? It's nice to have as a
| configuration / flag though as many project will be able to
| require it.
| erik_seaberg wrote:
| Last I looked, reflection was missing a lot of things like
| looking up a function by name.
| zimpenfish wrote:
| `MethodByName`[0] allows you to find a method on a struct
| but I'm assuming you must mean a generic package-level
| function rather than a method?
|
| [0] https://pkg.go.dev/reflect#Value.MethodByName
| erik_seaberg wrote:
| Yeah, you can look up a method but only if you already
| have the receiver's type (which you also can't look up by
| name, or by listing a package).
| zimpenfish wrote:
| You don't need the receiver's type because you're calling
| `MethodByName` on a `reflect.Value` which you can
| construct from any receiver.
|
| https://go.dev/play/p/hg-ugl7i0ei demonstrates this, I
| think?
| enneff wrote:
| Go's reflect package is not "missing" that feature, it is
| intentionally omitted as it defeats static analysis. In all
| my years writing Go (14 now), including some tricky
| metaprogramming, I have never needed to look up a function
| by name.
| erik_seaberg wrote:
| It creates busywork and ongoing maintenance issues in
| writing an interpreter. E.g.,
| https://pkg.go.dev/text/template requires every possible
| function to be manually registered, and that's not a good
| use of time.
| codeflo wrote:
| I still find Go too tolerant on this. It should scan your Git
| history and stash as well, and fail to compile if it finds any
| unused code there.
| SuperCuber wrote:
| That'd still be a bit too tolerant for my tastes. It should
| also scan your future commits.
| viscanti wrote:
| That'd still be a bit too tolerant for my tastes. It should
| have some kind of reinforcement signal that's stronger than
| just preventing the code from compiling. Some kind of shock
| therapy to help users learn and internalize things might be
| more appropriate here.
| lainga wrote:
| go vet -basilisk ./...
| yobert wrote:
| Really it needs to scan my coworkers commits as well. Can't
| be too careful.
| jerf wrote:
| Ah, I know the programming language for you:
| https://news.ycombinator.com/item?id=5002597
| mseepgood wrote:
| Could you please expand on this? Why would local Git stashes
| relate to code quality?
| erik_seaberg wrote:
| That comment might not be entirely serious.
| dewey wrote:
| Why not? What do you gain by keeping unused code in the
| codebase?
| lynndotpy wrote:
| When I was doing Genuary in Rust, I started with a template
| that I would add some helper functions too that I could
| anticipate I would need for the art. Easy ones like
| "xy_to_polar".
|
| I would do the first-time compile on this template and then
| add things iteratively.
|
| Go would be a nonstarter for this experimental/artistic code,
| because I would usually have dead code in my "toolbox" to
| iterate and experiment with until I liked the end product.
| dewey wrote:
| In reality you just comment out the block, in day-to-day
| work it really is a non-issue in my experience.
| 1zq wrote:
| I definitely don't want to keep unused code in version
| control, but for debugging or for working on things
| iteratively as the sibling comment said, my Go code usually
| ends up peppered with if 1==2 { ...some code
| to temporarily disable ... }
|
| "Just comment it out" is not always enough because when I
| want to quickly toggle the code on and off multiple times in
| a single session, it's not possible to do by only
| commenting/uncommenting that single block.
|
| Sometimes the code that's commented out contains the only
| reference to an imported package, and by commenting it out
| the autoformatter helpfully removes the import from the top
| of the file, so then uncommenting the line is not enough -- I
| need to go back and re-add the import.
|
| Or I may want to comment out this line x =
| f(a, b)
|
| but that makes a and b in turn become unused as well, and I
| need to travel up to wherever they're defined and comment
| them out too, and so on with anything else that became thus
| unused in that chain.
|
| And this is all suboptimal because the warning is also
| important when debugging. I do want to get warned about the
| unused code! Just let me compile.
| dewey wrote:
| > Sometimes the code that's commented out contains the only
| reference to an imported package, and by commenting it out
| the autoformatter helpfully removes the import from the top
| of the file, so then uncommenting the line is not enough --
| I need to go back and re-add the import.
|
| The same tool that removes your imports also adds them so
| there usually isn't an extra step.
| hoten wrote:
| Finds unreachable functions but does not seem to find dead code
| in sense of unused writes to a variable (unless I misread the
| article). Guess there's something else that already catches that
| in Go?
| nickcw wrote:
| One of the checkers in golangci-lint does this. I forget which
| one.
|
| golangci-lint rolls up lot of linters and checkers into a
| single binary.
|
| There is a config file too.
|
| https://github.com/golangci/golangci-lint
| mcdee wrote:
| It's called ineffassign
| jerf wrote:
| One of the mandatory ones, in my opinion. It's 3rd or 4th
| in terms of the linter that catches actual bugs for me
| before they are bugs, behind the number one errcheck and
| probably number two "unused", checking for unexported
| package-level variables that are unused. Go prohibits
| variables to be unused within a function but the compiler
| does not check that unexported package-level variables are
| used; this has helped me find a lot of places where I
| created a metric for something but never quite followed
| through to actually triggering it, log messages, bespoke
| errors, etc.
| klodolph wrote:
| Sweet. For some reason, this has been an ongoing problem with
| Go's tooling. I'm sure that there's some underlying reason why
| the dead code finding tools have kept getting deprecated or had
| various problems.
| v7n wrote:
| I will definitely use this! Also eager to see if it's fast enough
| to just run every time I save a bigger project.
| linuxftw wrote:
| The given example is a poor choice. Capitalized elements are
| public, and it's not safe to assume this code isn't being used as
| a library by other code outside the immediate repo.
| duskwuff wrote:
| The paragraph titled "Tests" addresses this.
|
| TL;DR: if your project is a library and an exported function
| looks like dead code, that means it isn't covered by any tests,
| and you should probably fix that.
| hydroxideOH- wrote:
| For those interested in this topic, I worked on improving dead
| code elimination in js_of_ocaml [1], a compiler from OCaml to
| JavaScript. The problem is more difficult in that case because of
| the indirection of OCaml's higher-order modules (functors).
|
| [1]: https://www.micahcantor.com/blog/js-of-ocaml-dead-code/
| superjared wrote:
| I inherited a go project that has two different commands under
| `cmd`, and it seems when I run this against one of those `main`s,
| it _incorrectly_ detects what it thinks as dead code that is used
| in the other command.
|
| Does anyone know how to work around this?
| icholy wrote:
| deadcode ./cmd/...
| superjared wrote:
| Thanks. That doesn't work for me (for some reason _nothing_
| is reported even though I know there's dead code) but it at
| least gives me a lead.
| vbezhenar wrote:
| IMO tree-shaking and similar stuff must be not an afterthought,
| but instead built into language and tooling from the day one.
| fransje26 wrote:
| Do you happen to know languages that implement that? I vaguely
| remember remember some Zig post on the topic, but I don't
| remember if it was an idea exploration, or an implemented
| feature.
| marcosdumay wrote:
| I can't name any language that was born with it.
|
| Most have it on the standard tools, but I don't think any
| language was born with it.
| rootext wrote:
| Dart compiler does it, as I know.
| kyrra wrote:
| One nice thing about go is that it ships with an AST package
| for parsing go code. It allows building of tooling really
| easily.
|
| https://pkg.go.dev/go/ast
| icholy wrote:
| Seems like you're conflating two separate things here. The Go
| compiler has a DCE pass which prevents unused code from ending
| up in the binary. The linked blog-post describes a tool for
| identifying unused code in your codebase. Two completely
| separate use-cases.
| BoppreH wrote:
| I agree that "changes the generated binary" and "displays
| information to the user" are separate use cases, but the
| compiler already does both. There's even a compiler error for
| unused imports and variables.
|
| Doesn't seem far-fetched to ask for the compiler to warn on
| unused types/functions too.
| icholy wrote:
| It's not that simple. The Go compiler uses a main package
| as its entry point when compiling a binary. A module can
| have a many main packages each using different subsets of
| the library packages.
|
| So there are two choices here: 1. The Go
| compiler complains about code which is unused by the
| current main package. 2. The Go compiler tries to
| find/compile all main packages every-time.
|
| Both of these options suck.
| mschuster91 wrote:
| And then, people start using reflection (Java), vtables/dynamic
| lookups (C, C++), eval (JS, PHP) and similar constructs (a
| _lot_ of languages have them), and suddenly tree-shaking falls
| apart.
|
| Tree shaking shouldn't be necessary in 2024 anyway.
| icholy wrote:
| Calling vtables "reflection" is a stretch.
| mschuster91 wrote:
| I was merely listing stuff that can break or severely
| restrict tree-shaking.
| baalimago wrote:
| I've used it a bit, didn't help me much more than "go vet" did.
| Also can't find code which is masked behind interfaces, unless
| I'm mistaken, which could amount of quite a lot of dead code.
| sugarpile wrote:
| > Also can't find code which is masked behind interfaces
|
| Is this not the Greeter interface example from the OP? The
| example finds an unused variant of an interface and all code
| only invoked by said unused implementations and marks all of it
| as deadcode.
| starttoaster wrote:
| I'm somewhat confused about this because I've had gopls installed
| with vscode for years now, and as far back as I can remember, it
| always gave me a little yellow underlining warning when I had
| unused functions or unreachable blocks of code inside `if` or
| `switch-case` statements. It would seem to me that this has
| existed for a long time, or maybe it just builds on top of what
| was already there?
| icholy wrote:
| I believe the checks you're referring are local to a package.
| It doesn't do whole program analysis like deadcode does.
___________________________________________________________________
(page generated 2024-01-23 23:01 UTC)