From 3d741349f5c7bf198a1c85acd374d1bee215f30e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Daniel=20Mart=C3=AD?= <mvdan@mvdan.cc>
Date: Thu, 14 Sep 2017 15:51:18 +0100
Subject: [PATCH] cmd/compile: collect reasons in inlining test
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

If we use -gcflags='-m -m', the compiler should give us a reason why a
func couldn't be inlined. Add the extra -m necessary for that extra info
and use it to give better test failures. For example, for the func in
the TODO:

	--- FAIL: TestIntendedInlining (1.53s)
		inl_test.go:104: runtime.nextFreeFast was not inlined: function too complex

We might increase the number of -m flags to get more information at some
later point, such as getting details on how close the func was to the
inlining budget.

Also started using regexes, as the output parsing is getting a bit too
complex for manual string handling.

While at it, also refactored the test to not buffer the entire output
into memory. This is fine in practice, but it won't scale well as we add
more packages or we depend more on the compiler's debugging output.

For example, "go build -a -gcflags='-m -m' std" prints nearly 40MB of
plaintext - and we only need to see the output line by line anyway.

Updates #21851.

Change-Id: I00986ff360eb56e4e9737b65a6be749ef8540643
Reviewed-on: https://go-review.googlesource.com/63810
Run-TryBot: Daniel Martí <mvdan@mvdan.cc>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Matthew Dempsky <mdempsky@google.com>
---
 src/cmd/compile/internal/gc/inl_test.go | 65 +++++++++++++++++--------
 1 file changed, 44 insertions(+), 21 deletions(-)

diff --git a/src/cmd/compile/internal/gc/inl_test.go b/src/cmd/compile/internal/gc/inl_test.go
index 9f5e3f2a4a..03dbd13b06 100644
--- a/src/cmd/compile/internal/gc/inl_test.go
+++ b/src/cmd/compile/internal/gc/inl_test.go
@@ -5,10 +5,13 @@
 package gc
 
 import (
-	"bytes"
+	"bufio"
 	"internal/testenv"
+	"io"
 	"os/exec"
+	"regexp"
 	"runtime"
+	"strings"
 	"testing"
 )
 
@@ -51,37 +54,57 @@ func TestIntendedInlining(t *testing.T) {
 		want["runtime"] = append(want["runtime"], "nextFreeFast")
 	}
 
-	m := make(map[string]bool)
+	notInlinedReason := make(map[string]string)
 	pkgs := make([]string, 0, len(want))
 	for pname, fnames := range want {
 		pkgs = append(pkgs, pname)
 		for _, fname := range fnames {
-			m[pname+"."+fname] = true
+			notInlinedReason[pname+"."+fname] = "unknown reason"
 		}
 	}
 
-	args := append([]string{"build", "-a", "-gcflags=-m"}, pkgs...)
+	args := append([]string{"build", "-a", "-gcflags=-m -m"}, pkgs...)
 	cmd := testenv.CleanCmdEnv(exec.Command(testenv.GoToolPath(t), args...))
-	out, err := cmd.CombinedOutput()
-	if err != nil {
-		t.Logf("%s", out)
-		t.Fatal(err)
-	}
-	lines := bytes.Split(out, []byte{'\n'})
+	pr, pw := io.Pipe()
+	cmd.Stdout = pw
+	cmd.Stderr = pw
+	cmdErr := make(chan error, 1)
+	go func() {
+		cmdErr <- cmd.Run()
+		pw.Close()
+	}()
+	scanner := bufio.NewScanner(pr)
 	curPkg := ""
-	for _, l := range lines {
-		if bytes.HasPrefix(l, []byte("# ")) {
-			curPkg = string(l[2:])
-		}
-		f := bytes.Split(l, []byte(": can inline "))
-		if len(f) < 2 {
+	canInline := regexp.MustCompile(`: can inline ([^ ]*)`)
+	cannotInline := regexp.MustCompile(`: cannot inline ([^ ]*): (.*)`)
+	for scanner.Scan() {
+		line := scanner.Text()
+		if strings.HasPrefix(line, "# ") {
+			curPkg = line[2:]
+			continue
+		}
+		if m := canInline.FindStringSubmatch(line); m != nil {
+			fname := m[1]
+			delete(notInlinedReason, curPkg+"."+fname)
+			continue
+		}
+		if m := cannotInline.FindStringSubmatch(line); m != nil {
+			fname, reason := m[1], m[2]
+			fullName := curPkg + "." + fname
+			if _, ok := notInlinedReason[fullName]; ok {
+				// cmd/compile gave us a reason why
+				notInlinedReason[fullName] = reason
+			}
 			continue
 		}
-		fn := bytes.TrimSpace(f[1])
-		delete(m, curPkg+"."+string(fn))
 	}
-
-	for s := range m {
-		t.Errorf("function %s not inlined", s)
+	if err := <-cmdErr; err != nil {
+		t.Fatal(err)
+	}
+	if err := scanner.Err(); err != nil {
+		t.Fatal(err)
+	}
+	for fullName, reason := range notInlinedReason {
+		t.Errorf("%s was not inlined: %s", fullName, reason)
 	}
 }