CC @adonovan

From my digging, here's what I've found -

in deadcode.go, we are utilizing ssautil's AllFunctions to get a list of functions from our program.

in ssautil/visit.go we have the following code:

switch mem := mem.(type) {
	case *ssa.Function:
		// Visit all package-level declared functions.
		function(mem)

	case *ssa.Type:
		exportedTypeHack(mem)
}

since the example struct is a type, and not a function, we call exportedTypeHack on it

exportedTypeHack, in turn, will only get named parameters if they are exported -- see ast.IsExported(t.Name()) on line 102 of go/ssa/ssautil/visit.go

this causes any functions belonging to the unexported example struct to not be included in our list returned by AllFunctions.

removing the call to ast.IsExported(t.Name()) will cause deadcode to produce the expected output for this specific file, and the tests outlined in testdata do all pass.

i myself am not positive that this will not induce negative side-effects elsewhere, but I figured i'd throw this out as a lead to someone who is more familiarized with this codebase than me

Change https://go.dev/cl/566421 mentions this issue: ssautil: remove export check on exportedTypeHack

The fix seems reasonable, but...

i myself am not positive that this will not induce negative side-effects elsewhere, but I figured i'd throw this out as a lead to someone who is more familiarized with this codebase than me

I share this concern. This will need an investigation to make sure the cure is not worse than this issue.

I will note that it is possible in some cases for methods on unexported structs to be correctly marked as deadcode. Running deadcode ./... on one of our codebases returns the following line (details redacted):

internal/{redacted}/file.go: unreachable func: unexportedStruct.ExportedMethod1
internal/{redacted}/{redacted}/file.go: unreachable func: unexportedStruct.unExportedMethod1

I unfortunately haven't been able to replicate what causes this to happen with a more succinct example that is sharable, but did want to clarify that in some cases these methods can actually be identified.

Surprisingly, the addition of a fmt.Println(example{}) allows the deadcode detector to correctly identify example.unUsed as dead code:

example/main.go

// type definitions and defined methods are unchanged from above 
func main() {
	fmt.Println(example{})
	example{}.Used()
	example{}.used()
	PublicExample{}.Used()
	PublicExample{}.used()
}

The output of deadcode ./.. now reports:

example/main.go:11:18: unreachable func: example.unUsed
example/main.go:17:24: unreachable func: PublicExample.UnUsed
example/main.go:21:24: unreachable func: PublicExample.unUsed

The output of deadcode -whylive scratchpad/example.example.UnUsed ./... now reports:

deadcode: scratchpad/example.example.UnUsed is reachable only through reflection

interestingly enough, when we call rta.Analyze in deadcode.go, we only check/set functions considered "Reachable" as true in the reachablePosn map, but it seems that when the fmt.Println() is added we start to consider almost everything reachable?

consider the following example, utilizing this print statement in main.go (not the best debugging tool I am aware :P)

example/main.go

type example struct{}

func (e example) UnUsed() {}
func (e example) Used()   {}

func (e example) unUsed() {}
func (e example) used()   {}

func main() {
	// fmt.Println(example{}) leaving this commented out in this example.
	example{}.Used()
	example{}.used()
}

as seen in deadcode.go

res := rta.Analyze(roots, *whyLiveFlag != "")

reachablePosn := make(map[token.Position]bool)

for fn := range res.Reachable {
if fn.Pos().IsValid() || fn.Name() == "init" {
	fmt.Println(fn.Name()) // added print statement here.
	reachablePosn[prog.Fset.Position(fn.Pos())] = true
}
}

will produce the following result -

used
init
main
Used
deadcode: function scratchpad/example.example.UnUsed is dead code

however, if we uncomment our println in main.go, it seems as though we print almost everything?
(edited for brevity, obviously)

pack
runqget
uvarint
Common
decref
full
releaseLockRank
newBucket
IsDirectIface
Kind
symPC
doaddtimer
chunkOf
next
addReflectOff
split
Chown
Width
HasName
Common
typeOff
Assign
alloc
findsghi
readLock
Seek
IfaceIndir
Unlock
typ
materializeGCProg
allocRange
Fchown$1
Release
casGToPreemptScan
newInlineUnwinder

(etc, etc, etc)

looking into why this happens now, but its quite interesting

Change https://go.dev/cl/567156 mentions this issue: cmd/deadcode: avoid ssautil.AllFunctions

Many thanks for the bug report and especially the minimal test case. The problem is indeed in AllFunctions, but the fix is not in that function: it's not to use it at all. AllFunctions performs a reachability analysis, which is not appropriate for the "denominator" of a dead code analysis. (The "numerator" is of course RTA, a more precise reachability analysis.) The correct denominator is the set of all source-level named functions, which is (happily) much cheaper to compute.

I mailed a fix in CL 567156.

[@thesilentg] Surprisingly, the addition of a fmt.Println(example{}) allows the deadcode detector to correctly identify example.unUsed as dead code:

AllFunction's use of a reachability analysis is the explanation of this paradoxical behavior: the print statement causes the example type to be added to RuntimeTypes, which causes all its methods to become reachable.

Thanks for the prompt investigation, explanation, and fix!