From 85a95658226904faa771be470dfb809b7e4aea22 Mon Sep 17 00:00:00 2001
From: Alan Donovan <adonovan@google.com>
Date: Thu, 11 Sep 2014 18:08:33 -0400
Subject: [PATCH] go/ssa: fix bug causing (manual) go/pointer stdlib test to
 crash.

The needMethods cache logic was wrong: it would treat any
previous call as a cache hit, even if 'skip' was true for that
call.  As a result it could fail to generate methods for some
'skip' types, i.e. anonymous structs.

LGTM=gri
R=gri
CC=golang-codereviews
https://golang.org/cl/144750043
---
 go/pointer/stdlib_test.go | 2 +-
 go/ssa/builder.go         | 8 ++++++--
 2 files changed, 7 insertions(+), 3 deletions(-)

diff --git a/go/pointer/stdlib_test.go b/go/pointer/stdlib_test.go
index c8380148..c5869ec5 100644
--- a/go/pointer/stdlib_test.go
+++ b/go/pointer/stdlib_test.go
@@ -54,7 +54,7 @@ func TestStdlib(t *testing.T) {
 	prog.BuildAll()
 
 	numPkgs := len(prog.AllPackages())
-	if want := 140; numPkgs < want {
+	if want := 240; numPkgs < want {
 		t.Errorf("Loaded only %d packages, want at least %d", numPkgs, want)
 	}
 
diff --git a/go/ssa/builder.go b/go/ssa/builder.go
index bcd5f096..a069c9e5 100644
--- a/go/ssa/builder.go
+++ b/go/ssa/builder.go
@@ -2307,9 +2307,13 @@ func (p *Package) needMethodsOf(T types.Type) {
 // Recursive case: skip => don't call makeMethods(T).
 func (p *Package) needMethods(T types.Type, skip bool) {
 	// Each package maintains its own set of types it has visited.
-	if p.needRTTI.Set(T, true) != nil {
-		return // already seen
+	if prevSkip, ok := p.needRTTI.At(T).(bool); ok {
+		// needMethods(T) was previously called
+		if !prevSkip || skip {
+			return // already seen, with same or false 'skip' value
+		}
 	}
+	p.needRTTI.Set(T, skip)
 
 	// Prune the recursion if we find a named or *named type
 	// belonging to another package.