Skip to content

Commit 4db4579

Browse files
committed
cmd/deadcode: avoid ssautil.AllFunctions
This change causes deadcode to enumerate the "universe set" of functions based on the source-level declarations, not the ssautil.AllFunctions helper. The former set is accurate (and also much cheaper to compute), whereas the latter is a mess (as its doc comment attests). In particular, it omits unexported methods of uninstantiated types, which are of course unreachable. This makes for some sense for whole-program analysis, but none at all for a dead code tool that specifically wants to know the useless functions. The new logic is pleasantly simpler. Also, a test. Fixes golang/go#65915 Change-Id: I7a4a64fd3da18d089044530911cdb2e3b42d0db5 Reviewed-on: https://go-review.googlesource.com/c/tools/+/567156 LUCI-TryBot-Result: Go LUCI <[email protected]> Reviewed-by: Robert Findley <[email protected]>
1 parent 9b58909 commit 4db4579

File tree

2 files changed

+72
-34
lines changed

2 files changed

+72
-34
lines changed

cmd/deadcode/deadcode.go

Lines changed: 28 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -132,16 +132,6 @@ func main() {
132132
log.Fatalf("packages contain errors")
133133
}
134134

135-
// Gather names of generated files.
136-
generated := make(map[string]bool)
137-
packages.Visit(initial, nil, func(p *packages.Package) {
138-
for _, file := range p.Syntax {
139-
if isGenerated(file) {
140-
generated[p.Fset.File(file.Pos()).Name()] = true
141-
}
142-
}
143-
})
144-
145135
// If -filter is unset, use first module (if available).
146136
if *filterFlag == "<module>" {
147137
if mod := initial[0].Module; mod != nil && mod.Path != "" {
@@ -169,6 +159,32 @@ func main() {
169159
roots = append(roots, main.Func("init"), main.Func("main"))
170160
}
171161

162+
// Gather all source-level functions,
163+
// as the user interface is expressed in terms of them.
164+
//
165+
// We ignore synthetic wrappers, and nested functions. Literal
166+
// functions passed as arguments to other functions are of
167+
// course address-taken and there exists a dynamic call of
168+
// that signature, so when they are unreachable, it is
169+
// invariably because the parent is unreachable.
170+
var sourceFuncs []*ssa.Function
171+
generated := make(map[string]bool)
172+
packages.Visit(initial, nil, func(p *packages.Package) {
173+
for _, file := range p.Syntax {
174+
for _, decl := range file.Decls {
175+
if decl, ok := decl.(*ast.FuncDecl); ok {
176+
obj := p.TypesInfo.Defs[decl.Name].(*types.Func)
177+
fn := prog.FuncValue(obj)
178+
sourceFuncs = append(sourceFuncs, fn)
179+
}
180+
}
181+
182+
if isGenerated(file) {
183+
generated[p.Fset.File(file.Pos()).Name()] = true
184+
}
185+
}
186+
})
187+
172188
// Compute the reachabilty from main.
173189
// (Build a call graph only for -whylive.)
174190
res := rta.Analyze(roots, *whyLiveFlag != "")
@@ -195,10 +211,7 @@ func main() {
195211
// is not dead, by showing a path to it from some root.
196212
if *whyLiveFlag != "" {
197213
targets := make(map[*ssa.Function]bool)
198-
for fn := range ssautil.AllFunctions(prog) {
199-
if fn.Synthetic != "" || fn.Object() == nil {
200-
continue // not a source-level named function
201-
}
214+
for _, fn := range sourceFuncs {
202215
if prettyName(fn, true) == *whyLiveFlag {
203216
targets[fn] = true
204217
}
@@ -263,26 +276,7 @@ func main() {
263276

264277
// Group unreachable functions by package path.
265278
byPkgPath := make(map[string]map[*ssa.Function]bool)
266-
for fn := range ssautil.AllFunctions(prog) {
267-
if fn.Synthetic != "" {
268-
continue // ignore synthetic wrappers etc
269-
}
270-
271-
// Use generic, as instantiations may not have a Pkg.
272-
if orig := fn.Origin(); orig != nil {
273-
fn = orig
274-
}
275-
276-
// Ignore unreachable nested functions.
277-
// Literal functions passed as arguments to other
278-
// functions are of course address-taken and there
279-
// exists a dynamic call of that signature, so when
280-
// they are unreachable, it is invariably because the
281-
// parent is unreachable.
282-
if fn.Parent() != nil {
283-
continue
284-
}
285-
279+
for _, fn := range sourceFuncs {
286280
posn := prog.Fset.Position(fn.Pos())
287281

288282
if !reachablePosn[posn] {
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
# Regression test for issue 65915: the enumeration of source-level
2+
# functions used the flawed ssautil.AllFunctions, causing it to
3+
# miss some unexported ones.
4+
5+
deadcode -filter= example.com
6+
7+
want "unreachable func: example.UnUsed"
8+
want "unreachable func: example.unUsed"
9+
want "unreachable func: PublicExample.UnUsed"
10+
want "unreachable func: PublicExample.unUsed"
11+
12+
-- go.mod --
13+
module example.com
14+
go 1.18
15+
16+
-- main.go --
17+
package main
18+
19+
type example struct{}
20+
21+
func (e example) UnUsed() {}
22+
23+
func (e example) Used() {}
24+
25+
func (e example) unUsed() {}
26+
27+
func (e example) used() {}
28+
29+
type PublicExample struct{}
30+
31+
func (p PublicExample) UnUsed() {}
32+
33+
func (p PublicExample) Used() {}
34+
35+
func (p PublicExample) unUsed() {}
36+
37+
func (p PublicExample) used() {}
38+
39+
func main() {
40+
example{}.Used()
41+
example{}.used()
42+
PublicExample{}.Used()
43+
PublicExample{}.used()
44+
}

0 commit comments

Comments
 (0)