Skip to content

Commit 7045d2e

Browse files
committed
go/analysis/passes/nilness: fix bug with MakeInterface(TypeParam)
An interface conversion such as any(x) can be a MakeInterface (if x is a non-interface type) or a ChangeInterface (if x has an interface type), and their nilabilities differ. If x's type is a type parameter, SSA uses MakeInterface, so we have to ascertain whether the type parameter is definitely or only maybe a concrete type in order to determine its nilability. This change required exposing NormalTerms to x/tools, and making it take the Underlying of its argument, so that NormalTerms(error) = NormalTerms(any) = []. Previously, NormalTerms(error) was [error]. Fixes golang/go#66835 Change-Id: Idf9c39afeaeab918b0f8e6288dd93570f7cb7081 Reviewed-on: https://go-review.googlesource.com/c/tools/+/578938 Reviewed-by: Tim King <[email protected]> LUCI-TryBot-Result: Go LUCI <[email protected]>
1 parent e1b14a1 commit 7045d2e

File tree

3 files changed

+72
-11
lines changed

3 files changed

+72
-11
lines changed

go/analysis/passes/nilness/nilness.go

Lines changed: 24 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import (
1414
"golang.org/x/tools/go/analysis/passes/buildssa"
1515
"golang.org/x/tools/go/analysis/passes/internal/analysisutil"
1616
"golang.org/x/tools/go/ssa"
17+
"golang.org/x/tools/internal/aliases"
1718
"golang.org/x/tools/internal/typeparams"
1819
)
1920

@@ -281,6 +282,7 @@ func (n nilness) String() string { return nilnessStrings[n+1] }
281282
// nilnessOf reports whether v is definitely nil, definitely not nil,
282283
// or unknown given the dominating stack of facts.
283284
func nilnessOf(stack []fact, v ssa.Value) nilness {
285+
284286
switch v := v.(type) {
285287
// unwrap ChangeInterface and Slice values recursively, to detect if underlying
286288
// values have any facts recorded or are otherwise known with regard to nilness.
@@ -296,6 +298,24 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
296298
if underlying := nilnessOf(stack, v.X); underlying != unknown {
297299
return underlying
298300
}
301+
case *ssa.MakeInterface:
302+
// A MakeInterface is non-nil unless its operand is a type parameter.
303+
tparam, ok := aliases.Unalias(v.X.Type()).(*types.TypeParam)
304+
if !ok {
305+
return isnonnil
306+
}
307+
308+
// A MakeInterface of a type parameter is non-nil if
309+
// the type parameter cannot be instantiated as an
310+
// interface type (#66835).
311+
if terms, err := typeparams.NormalTerms(tparam.Constraint()); err == nil && len(terms) > 0 {
312+
return isnonnil
313+
}
314+
315+
// If the type parameter can be instantiated as an
316+
// interface (and thus also as a concrete type),
317+
// we can't determine the nilness.
318+
299319
case *ssa.Slice:
300320
if underlying := nilnessOf(stack, v.X); underlying != unknown {
301321
return underlying
@@ -332,10 +352,10 @@ func nilnessOf(stack []fact, v ssa.Value) nilness {
332352
*ssa.IndexAddr,
333353
*ssa.MakeChan,
334354
*ssa.MakeClosure,
335-
*ssa.MakeInterface,
336355
*ssa.MakeMap,
337356
*ssa.MakeSlice:
338357
return isnonnil
358+
339359
case *ssa.Const:
340360
if v.IsNil() {
341361
return isnil // nil or zero value of a pointer-like type
@@ -424,6 +444,9 @@ func is[T any](x any) bool {
424444
}
425445

426446
func isNillable(t types.Type) bool {
447+
// TODO(adonovan): CoreType (+ case *Interface) looks wrong.
448+
// This should probably use Underlying, and handle TypeParam
449+
// by computing the union across its normal terms.
427450
switch t := typeparams.CoreType(t).(type) {
428451
case *types.Pointer,
429452
*types.Map,

go/analysis/passes/nilness/testdata/src/c/c.go

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,3 +12,43 @@ var g int
1212
func init() {
1313
g = instantiated[int](&g)
1414
}
15+
16+
// -- issue 66835 --
17+
18+
type Empty1 any
19+
type Empty2 any
20+
21+
// T may be instantiated with an interface type, so any(x) may be nil.
22+
func TypeParamInterface[T error](x T) {
23+
if any(x) == nil {
24+
print()
25+
}
26+
}
27+
28+
// T may not be instantiated with an interface type, so any(x) is non-nil
29+
func TypeParamTypeSetWithInt[T interface {
30+
error
31+
int
32+
}](x T) {
33+
if any(x) == nil { // want "impossible condition: non-nil == nil"
34+
print()
35+
}
36+
}
37+
38+
func TypeParamUnionEmptyEmpty[T Empty1 | Empty2](x T) {
39+
if any(x) == nil {
40+
print()
41+
}
42+
}
43+
44+
func TypeParamUnionEmptyInt[T Empty1 | int](x T) {
45+
if any(x) == nil {
46+
print()
47+
}
48+
}
49+
50+
func TypeParamUnionStringInt[T string | int](x T) {
51+
if any(x) == nil { // want "impossible condition: non-nil == nil"
52+
print()
53+
}
54+
}

internal/typeparams/coretype.go

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -7,8 +7,6 @@ package typeparams
77
import (
88
"fmt"
99
"go/types"
10-
11-
"golang.org/x/tools/internal/aliases"
1210
)
1311

1412
// CoreType returns the core type of T or nil if T does not have a core type.
@@ -20,7 +18,7 @@ func CoreType(T types.Type) types.Type {
2018
return U // for non-interface types,
2119
}
2220

23-
terms, err := _NormalTerms(U)
21+
terms, err := NormalTerms(U)
2422
if len(terms) == 0 || err != nil {
2523
// len(terms) -> empty type set of interface.
2624
// err != nil => U is invalid, exceeds complexity bounds, or has an empty type set.
@@ -66,7 +64,7 @@ func CoreType(T types.Type) types.Type {
6664
return ch
6765
}
6866

69-
// _NormalTerms returns a slice of terms representing the normalized structural
67+
// NormalTerms returns a slice of terms representing the normalized structural
7068
// type restrictions of a type, if any.
7169
//
7270
// For all types other than *types.TypeParam, *types.Interface, and
@@ -96,23 +94,23 @@ func CoreType(T types.Type) types.Type {
9694
// expands to ~string|~[]byte|int|string, which reduces to ~string|~[]byte|int,
9795
// which when intersected with C (~string|~int) yields ~string|int.
9896
//
99-
// _NormalTerms computes these expansions and reductions, producing a
97+
// NormalTerms computes these expansions and reductions, producing a
10098
// "normalized" form of the embeddings. A structural restriction is normalized
10199
// if it is a single union containing no interface terms, and is minimal in the
102100
// sense that removing any term changes the set of types satisfying the
103101
// constraint. It is left as a proof for the reader that, modulo sorting, there
104102
// is exactly one such normalized form.
105103
//
106-
// Because the minimal representation always takes this form, _NormalTerms
104+
// Because the minimal representation always takes this form, NormalTerms
107105
// returns a slice of tilde terms corresponding to the terms of the union in
108106
// the normalized structural restriction. An error is returned if the type is
109107
// invalid, exceeds complexity bounds, or has an empty type set. In the latter
110-
// case, _NormalTerms returns ErrEmptyTypeSet.
108+
// case, NormalTerms returns ErrEmptyTypeSet.
111109
//
112-
// _NormalTerms makes no guarantees about the order of terms, except that it
110+
// NormalTerms makes no guarantees about the order of terms, except that it
113111
// is deterministic.
114-
func _NormalTerms(typ types.Type) ([]*types.Term, error) {
115-
switch typ := aliases.Unalias(typ).(type) {
112+
func NormalTerms(typ types.Type) ([]*types.Term, error) {
113+
switch typ := typ.Underlying().(type) {
116114
case *types.TypeParam:
117115
return StructuralTerms(typ)
118116
case *types.Union:

0 commit comments

Comments
 (0)