gopls/internal/golang: Assembly: support package level var and init

This CL offers the "Browse Assembly" code action when the selection
is within a package-level var initializer, or a source-level init
function.

+ Test

Change-Id: Ic0fcf321765027df0c11fb7269a1aedf971814fc
Reviewed-on: https://go-review.googlesource.com/c/tools/+/652197
Auto-Submit: Alan Donovan <[email protected]>
LUCI-TryBot-Result: Go LUCI <[email protected]>
Reviewed-by: Robert Findley <[email protected]>

Author: adonovan

gopls/internal/golang/assembly.go

@@ -29,6 +29,8 @@ import (
 // AssemblyHTML returns an HTML document containing an assembly listing of the selected function.
 //
 // TODO(adonovan): cross-link jumps and block labels, like github.com/aclements/objbrowse.
+//
+// See gopls/internal/test/integration/misc/webserver_test.go for tests.
 func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.ResponseWriter, pkg *cache.Package, symbol string, web Web) {
 	// Prepare to compile the package with -S, and capture its stderr stream.
 	inv, cleanupInvocation, err := snapshot.GoCommandInvocation(cache.NoNetwork, pkg.Metadata().CompiledGoFiles[0].DirPath(), "build", []string{"-gcflags=-S", "."})
@@ -72,11 +74,26 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.Response
 		flusher.Flush()
 	}
 
+	// At this point errors must be reported by writing HTML.
+	// To do this, set "status" return early.
+
+	var buf bytes.Buffer
+	status := "Reload the page to recompile."
+	defer func() {
+		// Update the "Compiling..." message.
+		fmt.Fprintf(&buf, `
+</pre>
+<script>
+document.getElementById('compiling').innerText = %q;
+</script>
+</body>`, status)
+		w.Write(buf.Bytes())
+	}()
+
 	// Compile the package.
 	_, stderr, err, _ := snapshot.View().GoCommandRunner().RunRaw(ctx, *inv)
 	if err != nil {
-		// e.g. won't compile
-		http.Error(w, err.Error(), http.StatusInternalServerError)
+		status = fmt.Sprintf("compilation failed: %v", err)
 		return
 	}
 
@@ -96,7 +113,6 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.Response
 	//
 	// Allow matches of symbol, symbol.func1, symbol.deferwrap, etc.
 	on := false
-	var buf bytes.Buffer
 	for line := range strings.SplitSeq(content, "\n") {
 		// start of function symbol?
 		if strings.Contains(line, " STEXT ") {
@@ -125,14 +141,4 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.Response
 		}
 		buf.WriteByte('\n')
 	}
-
-	// Update the "Compiling..." message.
-	buf.WriteString(`
-</pre>
-<script>
-document.getElementById('compiling').innerText = 'Reload the page to recompile.';
-</script>
-</body>`)
-
-	w.Write(buf.Bytes())
 }

gopls/internal/golang/codeaction.go

@@ -945,44 +945,66 @@ func goAssembly(ctx context.Context, req *codeActionsRequest) error {
 	// directly to (say) a lambda of interest.
 	// Perhaps we could scroll to STEXT for the innermost
 	// enclosing nested function?
-	path, _ := astutil.PathEnclosingInterval(req.pgf.File, req.start, req.end)
-	if len(path) >= 2 { // [... FuncDecl File]
-		if decl, ok := path[len(path)-2].(*ast.FuncDecl); ok {
-			if fn, ok := req.pkg.TypesInfo().Defs[decl.Name].(*types.Func); ok {
-				sig := fn.Signature()
-
-				// Compute the linker symbol of the enclosing function.
-				var sym strings.Builder
-				if fn.Pkg().Name() == "main" {
-					sym.WriteString("main")
-				} else {
-					sym.WriteString(fn.Pkg().Path())
-				}
-				sym.WriteString(".")
-				if sig.Recv() != nil {
-					if isPtr, named := typesinternal.ReceiverNamed(sig.Recv()); named != nil {
-						if isPtr {
-							fmt.Fprintf(&sym, "(*%s)", named.Obj().Name())
-						} else {
-							sym.WriteString(named.Obj().Name())
+
+	// Compute the linker symbol of the enclosing function or var initializer.
+	var sym strings.Builder
+	if pkg := req.pkg.Types(); pkg.Name() == "main" {
+		sym.WriteString("main")
+	} else {
+		sym.WriteString(pkg.Path())
+	}
+	sym.WriteString(".")
+
+	curSel, _ := req.pgf.Cursor.FindPos(req.start, req.end)
+	for cur := range curSel.Ancestors((*ast.FuncDecl)(nil), (*ast.ValueSpec)(nil)) {
+		var name string // in command title
+		switch node := cur.Node().(type) {
+		case *ast.FuncDecl:
+			// package-level func or method
+			if fn, ok := req.pkg.TypesInfo().Defs[node.Name].(*types.Func); ok &&
+				fn.Name() != "_" { // blank functions are not compiled
+
+				// Source-level init functions are compiled (along with
+				// package-level var initializers) in into a single pkg.init
+				// function, so this falls out of the logic below.
+
+				if sig := fn.Signature(); sig.TypeParams() == nil && sig.RecvTypeParams() == nil { // generic => no assembly
+					if sig.Recv() != nil {
+						if isPtr, named := typesinternal.ReceiverNamed(sig.Recv()); named != nil {
+							if isPtr {
+								fmt.Fprintf(&sym, "(*%s)", named.Obj().Name())
+							} else {
+								sym.WriteString(named.Obj().Name())
+							}
+							sym.WriteByte('.')
 						}
-						sym.WriteByte('.')
 					}
+					sym.WriteString(fn.Name())
+
+					name = node.Name.Name // success
 				}
-				sym.WriteString(fn.Name())
-
-				if fn.Name() != "_" && // blank functions are not compiled
-					(fn.Name() != "init" || sig.Recv() != nil) && // init functions aren't linker functions
-					sig.TypeParams() == nil && sig.RecvTypeParams() == nil { // generic => no assembly
-					cmd := command.NewAssemblyCommand(
-						fmt.Sprintf("Browse %s assembly for %s", view.GOARCH(), decl.Name),
-						view.ID(),
-						string(req.pkg.Metadata().ID),
-						sym.String())
-					req.addCommandAction(cmd, false)
+			}
+
+		case *ast.ValueSpec:
+			// package-level var initializer?
+			if len(node.Names) > 0 && len(node.Values) > 0 {
+				v := req.pkg.TypesInfo().Defs[node.Names[0]]
+				if v != nil && typesinternal.IsPackageLevel(v) {
+					sym.WriteString("init")
+					name = "package initializer" // success
 				}
 			}
 		}
+
+		if name != "" {
+			cmd := command.NewAssemblyCommand(
+				fmt.Sprintf("Browse %s assembly for %s", view.GOARCH(), name),
+				view.ID(),
+				string(req.pkg.Metadata().ID),
+				sym.String())
+			req.addCommandAction(cmd, false)
+			break
+		}
 	}
 	return nil
 }

gopls/internal/test/integration/misc/webserver_test.go

@@ -520,43 +520,57 @@ module example.com
 -- a/a.go --
 package a
 
-func f() {
+func f(x int) int {
 	println("hello")
 	defer println("world")
+	return x
 }
 
 func g() {
 	println("goodbye")
 }
+
+var v = [...]int{
+	f(123),
+	f(456),
+}
+
+func init() {
+	f(789)
+}
 `
 	Run(t, files, func(t *testing.T, env *Env) {
 		env.OpenFile("a/a.go")
 
-		// Invoke the "Browse assembly" code action to start the server.
-		loc := env.RegexpSearch("a/a.go", "println")
-		actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger)
-		if err != nil {
-			t.Fatalf("CodeAction: %v", err)
-		}
-		action, err := codeActionByKind(actions, settings.GoAssembly)
-		if err != nil {
-			t.Fatal(err)
-		}
+		asmFor := func(pattern string) []byte {
+			// Invoke the "Browse assembly" code action to start the server.
+			loc := env.RegexpSearch("a/a.go", pattern)
+			actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger)
+			if err != nil {
+				t.Fatalf("CodeAction: %v", err)
+			}
+			action, err := codeActionByKind(actions, settings.GoAssembly)
+			if err != nil {
+				t.Fatal(err)
+			}
 
-		// Execute the command.
-		// Its side effect should be a single showDocument request.
-		params := &protocol.ExecuteCommandParams{
-			Command:   action.Command.Command,
-			Arguments: action.Command.Arguments,
-		}
-		var result command.DebuggingResult
-		collectDocs := env.Awaiter.ListenToShownDocuments()
-		env.ExecuteCommand(params, &result)
-		doc := shownDocument(t, collectDocs(), "http:")
-		if doc == nil {
-			t.Fatalf("no showDocument call had 'file:' prefix")
+			// Execute the command.
+			// Its side effect should be a single showDocument request.
+			params := &protocol.ExecuteCommandParams{
+				Command:   action.Command.Command,
+				Arguments: action.Command.Arguments,
+			}
+			var result command.DebuggingResult
+			collectDocs := env.Awaiter.ListenToShownDocuments()
+			env.ExecuteCommand(params, &result)
+			doc := shownDocument(t, collectDocs(), "http:")
+			if doc == nil {
+				t.Fatalf("no showDocument call had 'file:' prefix")
+			}
+			t.Log("showDocument(package doc) URL:", doc.URI)
+
+			return get(t, doc.URI)
 		}
-		t.Log("showDocument(package doc) URL:", doc.URI)
 
 		// Get the report and do some minimal checks for sensible results.
 		//
@@ -567,23 +581,42 @@ func g() {
 		// (e.g. uses JAL for CALL, or BL<cc> for RET).
 		// We conservatively test only on the two most popular
 		// architectures.
-		report := get(t, doc.URI)
-		checkMatch(t, true, report, `TEXT.*example.com/a.f`)
-		switch runtime.GOARCH {
-		case "amd64", "arm64":
-			checkMatch(t, true, report, `CALL	runtime.printlock`)
-			checkMatch(t, true, report, `CALL	runtime.printstring`)
-			checkMatch(t, true, report, `CALL	runtime.printunlock`)
-			checkMatch(t, true, report, `CALL	example.com/a.f.deferwrap1`)
-			checkMatch(t, true, report, `RET`)
-			checkMatch(t, true, report, `CALL	runtime.morestack_noctxt`)
+		{
+			report := asmFor("println")
+			checkMatch(t, true, report, `TEXT.*example.com/a.f`)
+			switch runtime.GOARCH {
+			case "amd64", "arm64":
+				checkMatch(t, true, report, `CALL	runtime.printlock`)
+				checkMatch(t, true, report, `CALL	runtime.printstring`)
+				checkMatch(t, true, report, `CALL	runtime.printunlock`)
+				checkMatch(t, true, report, `CALL	example.com/a.f.deferwrap1`)
+				checkMatch(t, true, report, `RET`)
+				checkMatch(t, true, report, `CALL	runtime.morestack_noctxt`)
+			}
+
+			// Nested functions are also shown.
+			checkMatch(t, true, report, `TEXT.*example.com/a.f.deferwrap1`)
+
+			// But other functions are not.
+			checkMatch(t, false, report, `TEXT.*example.com/a.g`)
 		}
 
-		// Nested functions are also shown.
-		checkMatch(t, true, report, `TEXT.*example.com/a.f.deferwrap1`)
+		// Check that code in a package-level var initializer is found too.
+		{
+			report := asmFor(`f\(123\)`)
+			checkMatch(t, true, report, `TEXT.*example.com/a.init`)
+			checkMatch(t, true, report, `MOV.	\$123`)
+			checkMatch(t, true, report, `MOV.	\$456`)
+			checkMatch(t, true, report, `CALL	example.com/a.f`)
+		}
 
-		// But other functions are not.
-		checkMatch(t, false, report, `TEXT.*example.com/a.g`)
+		// And code in a source-level init function.
+		{
+			report := asmFor(`f\(789\)`)
+			checkMatch(t, true, report, `TEXT.*example.com/a.init`)
+			checkMatch(t, true, report, `MOV.	\$789`)
+			checkMatch(t, true, report, `CALL	example.com/a.f`)
+		}
 	})
 }