go/types, types2: verify stateMask transitions in debug mode

Recently, we've changed the representation of Named type state from
an integer to a bit mask, which is a bit more complicated. To make
sure we uphold state invariants, we are adding a verification step
on each state transition.

This uncovered a few places where we do not uphold the transition
invariants; those are patched in this CL.

Change-Id: I76569e4326b2d362d7a1f078641029ffb3dca531
Reviewed-on: https://go-review.googlesource.com/c/go/+/714241
Auto-Submit: Mark Freeman <markfreeman@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>

diff --git a/src/cmd/compile/internal/types2/named.go b/src/cmd/compile/internal/types2/named.go
index 9f99c568d74dd7c626372aaebfc99140b4e14a0a..11d7ba1734e72acd466c227dd92d5891de8f01e7 100644 (file)
--- a/src/cmd/compile/internal/types2/named.go
+++ b/src/cmd/compile/internal/types2/named.go
@@ -215,7 +215,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
 // All others:
 // Effectively, nothing happens.
 func (n *Named) unpack() *Named {
-       if n.stateHas(unpacked | lazyLoaded) { // avoid locking below
+       if n.stateHas(lazyLoaded | unpacked) { // avoid locking below
                return n
        }
 
@@ -225,7 +225,7 @@ func (n *Named) unpack() *Named {
        defer n.mu.Unlock()
 
        // only atomic for consistency; we are holding the mutex
-       if n.stateHas(unpacked | lazyLoaded) {
+       if n.stateHas(lazyLoaded | unpacked) {
                return n
        }
 
@@ -243,10 +243,10 @@ func (n *Named) unpack() *Named {
                n.tparams = orig.tparams
 
                if len(orig.methods) == 0 {
-                       n.setState(unpacked | hasMethods) // nothing further to do
+                       n.setState(lazyLoaded | unpacked | hasMethods) // nothing further to do
                        n.inst.ctxt = nil
                } else {
-                       n.setState(unpacked)
+                       n.setState(lazyLoaded | unpacked)
                }
                return n
        }
@@ -275,19 +275,36 @@ func (n *Named) unpack() *Named {
                }
        }
 
-       n.setState(unpacked | hasMethods)
+       n.setState(lazyLoaded | unpacked | hasMethods)
        return n
 }
 
 // stateHas atomically determines whether the current state includes any active bit in sm.
-func (n *Named) stateHas(sm stateMask) bool {
-       return atomic.LoadUint32(&n.state_)&uint32(sm) != 0
+func (n *Named) stateHas(m stateMask) bool {
+       return stateMask(atomic.LoadUint32(&n.state_))&m != 0
 }
 
 // setState atomically sets the current state to include each active bit in sm.
 // Must only be called while holding n.mu.
-func (n *Named) setState(sm stateMask) {
-       atomic.OrUint32(&n.state_, uint32(sm))
+func (n *Named) setState(m stateMask) {
+       atomic.OrUint32(&n.state_, uint32(m))
+       // verify state transitions
+       if debug {
+               m := stateMask(atomic.LoadUint32(&n.state_))
+               u := m&unpacked != 0
+               // unpacked => lazyLoaded
+               if u {
+                       assert(m&lazyLoaded != 0)
+               }
+               // hasMethods => unpacked
+               if m&hasMethods != 0 {
+                       assert(u)
+               }
+               // hasUnder => unpacked
+               if m&hasUnder != 0 {
+                       assert(u)
+               }
+       }
 }
 
 // newNamed is like NewNamed but with a *Checker receiver.
@@ -503,7 +520,7 @@ func (t *Named) SetUnderlying(u Type) {
 
        t.fromRHS = u
        t.allowNilRHS = false
-       t.setState(unpacked | hasMethods) // TODO(markfreeman): Why hasMethods?
+       t.setState(lazyLoaded | unpacked | hasMethods) // TODO(markfreeman): Why hasMethods?
 
        t.underlying = u
        t.allowNilUnderlying = false

diff --git a/src/go/types/named.go b/src/go/types/named.go
index 7b9856155f462ec47010f39b3e361e1c03da3908..564c1be3e0ef70056c8d55d01a5c8fb8af2fb865 100644 (file)
--- a/src/go/types/named.go
+++ b/src/go/types/named.go
@@ -218,7 +218,7 @@ func NewNamed(obj *TypeName, underlying Type, methods []*Func) *Named {
 // All others:
 // Effectively, nothing happens.
 func (n *Named) unpack() *Named {
-       if n.stateHas(unpacked | lazyLoaded) { // avoid locking below
+       if n.stateHas(lazyLoaded | unpacked) { // avoid locking below
                return n
        }
 
@@ -228,7 +228,7 @@ func (n *Named) unpack() *Named {
        defer n.mu.Unlock()
 
        // only atomic for consistency; we are holding the mutex
-       if n.stateHas(unpacked | lazyLoaded) {
+       if n.stateHas(lazyLoaded | unpacked) {
                return n
        }
 
@@ -246,10 +246,10 @@ func (n *Named) unpack() *Named {
                n.tparams = orig.tparams
 
                if len(orig.methods) == 0 {
-                       n.setState(unpacked | hasMethods) // nothing further to do
+                       n.setState(lazyLoaded | unpacked | hasMethods) // nothing further to do
                        n.inst.ctxt = nil
                } else {
-                       n.setState(unpacked)
+                       n.setState(lazyLoaded | unpacked)
                }
                return n
        }
@@ -278,19 +278,36 @@ func (n *Named) unpack() *Named {
                }
        }
 
-       n.setState(unpacked | hasMethods)
+       n.setState(lazyLoaded | unpacked | hasMethods)
        return n
 }
 
 // stateHas atomically determines whether the current state includes any active bit in sm.
-func (n *Named) stateHas(sm stateMask) bool {
-       return atomic.LoadUint32(&n.state_)&uint32(sm) != 0
+func (n *Named) stateHas(m stateMask) bool {
+       return stateMask(atomic.LoadUint32(&n.state_))&m != 0
 }
 
 // setState atomically sets the current state to include each active bit in sm.
 // Must only be called while holding n.mu.
-func (n *Named) setState(sm stateMask) {
-       atomic.OrUint32(&n.state_, uint32(sm))
+func (n *Named) setState(m stateMask) {
+       atomic.OrUint32(&n.state_, uint32(m))
+       // verify state transitions
+       if debug {
+               m := stateMask(atomic.LoadUint32(&n.state_))
+               u := m&unpacked != 0
+               // unpacked => lazyLoaded
+               if u {
+                       assert(m&lazyLoaded != 0)
+               }
+               // hasMethods => unpacked
+               if m&hasMethods != 0 {
+                       assert(u)
+               }
+               // hasUnder => unpacked
+               if m&hasUnder != 0 {
+                       assert(u)
+               }
+       }
 }
 
 // newNamed is like NewNamed but with a *Checker receiver.
@@ -506,7 +523,7 @@ func (t *Named) SetUnderlying(u Type) {
 
        t.fromRHS = u
        t.allowNilRHS = false
-       t.setState(unpacked | hasMethods) // TODO(markfreeman): Why hasMethods?
+       t.setState(lazyLoaded | unpacked | hasMethods) // TODO(markfreeman): Why hasMethods?
 
        t.underlying = u
        t.allowNilUnderlying = false