x/tools/go/analysis/passes/nilness: reports wrong diagnostic

[kfcss]

Go version

go version go1.24.2 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/<REDACTED>/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/<REDACTED>/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/8k/<REDACTED>/T/go-build3796509342=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/<REDACTED>/thirdparty/go-xtools/go.mod'
GOMODCACHE='/Users/<REDACTED>/go/pkg/mod'
GONOPROXY='none'
GONOSUMDB='css.com'
GOOS='darwin'
GOPATH='/Users/<REDACTED>/go'
GOPRIVATE='css.com'
GOPROXY='https://proxy.golang.org,<REDACTED>,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/<REDACTED>/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.2'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

I checked out commit f76b112f11ae44045f70b22a3d0dc0627f72ae10 of https://github.com/golang/tools.

Added the following to the end of https://github.com/golang/tools/blob/master/go/analysis/passes/nilness/testdata/src/a/a.go:

func f20(s1 *string) func() {
	return func() {
		isNilOrFoo := s1 == nil
		if !isNilOrFoo {
			isNilOrFoo = isNilOrFoo || *s1 == "foo"
		}
	}
}

Went to the the path go/analysis/passes/nilness and run go test.

What did you see happen?

I saw a diagnostic reported.

--- FAIL: Test (0.22s)
    analysistest.go:630: a/a.go:309:20: unexpected diagnostic: impossible condition: non-nil == nil

What did you expect to see?

No diagnostic should be reported.

If isNilOrFoo = isNilOrFoo || *s1 == "foo" is changed to isNilOrFoo = *s1 == "foo" no diagnostic is reported.

In fact isNilOrFoo is known to be false in isNilOrFoo || *s1 == "foo", which is probably the cause of the diagnostic. But it is very confusing that a diagnostic is reported on s1 == nil.

I don't know how this could be improved, but wanted to provide this as a test case as it caused confusion.

[gabyhelp]

Related Issues

• x/tools/go/analysis/passes/nilness: spurious "impossible/tautological condition" with cond := a != nil; if cond {...}; if cond { ... } #70381
• x/tools/go/analysis/passes/nilness: bogus "impossible condition: non-nil == nil" that is actually possible #66835 (closed)
• x/tools/go/analysis/passes/nilness: not report slice as non-nil #45177 (closed)
• x/tools/go/analysis/passes/nilness: missing location for error in embedded struct #51986 (closed)
• import/path: core #67494 (closed)
• everything: fail to check err == nil when assigning explicit type to `error` #56930 (closed)
• x/tools/go/analysis/passes/nilness: no position information with error #31008
• x/tools/gopls: support non-vet analyses #36639 (closed)
• nil literal should not be parsed as ast.Ident #28918 (closed)
• Nil is not nill in some cases #70364 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[adonovan]

This is a dup of #70381; see #70381 (comment) for explanation. In short,
the analyzer can't distinguish the actual program:

func f20(s1 *string) func() {
	return func() {
		isNilOrFoo := s1 == nil
		if !isNilOrFoo {
			isNilOrFoo = isNilOrFoo || *s1 == "foo"
		}
	}
}

from this alternative program:

func f20(s1 *string) func() {
	return func() {
		isNilOrFoo := s1 == nil
		if !isNilOrFoo {
			isNilOrFoo = s1 == nil || *s1 == "foo"
		}
	}
}

because the SSA representation for both is essentially identical. This causes it to report an error for what it thinks is the second occurrence of the s1 == nil condition in the second program, which would be a correct diagnostic---except that the s1 == nil.

The fix is to record additional bookkeeping in the SSA representation (such as enabling the GlobalDebug flag, but that's too expensive). Needs some careful thought. Apologies for the confusion.

I'm going to close this issue as a dup, but mention it from the canonical issue.