Skip to content

Commit e890c1f

Browse files
adonovangopherbot
adonovan
authored and
gopherbot
committed
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 <adonovan@google.com> LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com> Reviewed-by: Robert Findley <rfindley@google.com>
1 parent 6d4af1e commit e890c1f

File tree

3 files changed

+144
-83
lines changed

3 files changed

+144
-83
lines changed

‎gopls/internal/golang/assembly.go‎

Lines changed: 19 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,8 @@ import (
2929
// AssemblyHTML returns an HTML document containing an assembly listing of the selected function.
3030
//
3131
// TODO(adonovan): cross-link jumps and block labels, like github.com/aclements/objbrowse.
32+
//
33+
// See gopls/internal/test/integration/misc/webserver_test.go for tests.
3234
func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.ResponseWriter, pkg *cache.Package, symbol string, web Web) {
3335
// Prepare to compile the package with -S, and capture its stderr stream.
3436
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
7274
flusher.Flush()
7375
}
7476

77+
// At this point errors must be reported by writing HTML.
78+
// To do this, set "status" return early.
79+
80+
var buf bytes.Buffer
81+
status := "Reload the page to recompile."
82+
defer func() {
83+
// Update the "Compiling..." message.
84+
fmt.Fprintf(&buf, `
85+
</pre>
86+
<script>
87+
document.getElementById('compiling').innerText = %q;
88+
</script>
89+
</body>`, status)
90+
w.Write(buf.Bytes())
91+
}()
92+
7593
// Compile the package.
7694
_, stderr, err, _ := snapshot.View().GoCommandRunner().RunRaw(ctx, *inv)
7795
if err != nil {
78-
// e.g. won't compile
79-
http.Error(w, err.Error(), http.StatusInternalServerError)
96+
status = fmt.Sprintf("compilation failed: %v", err)
8097
return
8198
}
8299

@@ -96,7 +113,6 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.Response
96113
//
97114
// Allow matches of symbol, symbol.func1, symbol.deferwrap, etc.
98115
on := false
99-
var buf bytes.Buffer
100116
for line := range strings.SplitSeq(content, "\n") {
101117
// start of function symbol?
102118
if strings.Contains(line, " STEXT ") {
@@ -125,14 +141,4 @@ func AssemblyHTML(ctx context.Context, snapshot *cache.Snapshot, w http.Response
125141
}
126142
buf.WriteByte('\n')
127143
}
128-
129-
// Update the "Compiling..." message.
130-
buf.WriteString(`
131-
</pre>
132-
<script>
133-
document.getElementById('compiling').innerText = 'Reload the page to recompile.';
134-
</script>
135-
</body>`)
136-
137-
w.Write(buf.Bytes())
138144
}

‎gopls/internal/golang/codeaction.go‎

Lines changed: 54 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -945,44 +945,66 @@ func goAssembly(ctx context.Context, req *codeActionsRequest) error {
945945
// directly to (say) a lambda of interest.
946946
// Perhaps we could scroll to STEXT for the innermost
947947
// enclosing nested function?
948-
path, _ := astutil.PathEnclosingInterval(req.pgf.File, req.start, req.end)
949-
if len(path) >= 2 { // [... FuncDecl File]
950-
if decl, ok := path[len(path)-2].(*ast.FuncDecl); ok {
951-
if fn, ok := req.pkg.TypesInfo().Defs[decl.Name].(*types.Func); ok {
952-
sig := fn.Signature()
953-
954-
// Compute the linker symbol of the enclosing function.
955-
var sym strings.Builder
956-
if fn.Pkg().Name() == "main" {
957-
sym.WriteString("main")
958-
} else {
959-
sym.WriteString(fn.Pkg().Path())
960-
}
961-
sym.WriteString(".")
962-
if sig.Recv() != nil {
963-
if isPtr, named := typesinternal.ReceiverNamed(sig.Recv()); named != nil {
964-
if isPtr {
965-
fmt.Fprintf(&sym, "(*%s)", named.Obj().Name())
966-
} else {
967-
sym.WriteString(named.Obj().Name())
948+
949+
// Compute the linker symbol of the enclosing function or var initializer.
950+
var sym strings.Builder
951+
if pkg := req.pkg.Types(); pkg.Name() == "main" {
952+
sym.WriteString("main")
953+
} else {
954+
sym.WriteString(pkg.Path())
955+
}
956+
sym.WriteString(".")
957+
958+
curSel, _ := req.pgf.Cursor.FindPos(req.start, req.end)
959+
for cur := range curSel.Ancestors((*ast.FuncDecl)(nil), (*ast.ValueSpec)(nil)) {
960+
var name string // in command title
961+
switch node := cur.Node().(type) {
962+
case *ast.FuncDecl:
963+
// package-level func or method
964+
if fn, ok := req.pkg.TypesInfo().Defs[node.Name].(*types.Func); ok &&
965+
fn.Name() != "_" { // blank functions are not compiled
966+
967+
// Source-level init functions are compiled (along with
968+
// package-level var initializers) in into a single pkg.init
969+
// function, so this falls out of the logic below.
970+
971+
if sig := fn.Signature(); sig.TypeParams() == nil && sig.RecvTypeParams() == nil { // generic => no assembly
972+
if sig.Recv() != nil {
973+
if isPtr, named := typesinternal.ReceiverNamed(sig.Recv()); named != nil {
974+
if isPtr {
975+
fmt.Fprintf(&sym, "(*%s)", named.Obj().Name())
976+
} else {
977+
sym.WriteString(named.Obj().Name())
978+
}
979+
sym.WriteByte('.')
968980
}
969-
sym.WriteByte('.')
970981
}
982+
sym.WriteString(fn.Name())
983+
984+
name = node.Name.Name // success
971985
}
972-
sym.WriteString(fn.Name())
973-
974-
if fn.Name() != "_" && // blank functions are not compiled
975-
(fn.Name() != "init" || sig.Recv() != nil) && // init functions aren't linker functions
976-
sig.TypeParams() == nil && sig.RecvTypeParams() == nil { // generic => no assembly
977-
cmd := command.NewAssemblyCommand(
978-
fmt.Sprintf("Browse %s assembly for %s", view.GOARCH(), decl.Name),
979-
view.ID(),
980-
string(req.pkg.Metadata().ID),
981-
sym.String())
982-
req.addCommandAction(cmd, false)
986+
}
987+
988+
case *ast.ValueSpec:
989+
// package-level var initializer?
990+
if len(node.Names) > 0 && len(node.Values) > 0 {
991+
v := req.pkg.TypesInfo().Defs[node.Names[0]]
992+
if v != nil && typesinternal.IsPackageLevel(v) {
993+
sym.WriteString("init")
994+
name = "package initializer" // success
983995
}
984996
}
985997
}
998+
999+
if name != "" {
1000+
cmd := command.NewAssemblyCommand(
1001+
fmt.Sprintf("Browse %s assembly for %s", view.GOARCH(), name),
1002+
view.ID(),
1003+
string(req.pkg.Metadata().ID),
1004+
sym.String())
1005+
req.addCommandAction(cmd, false)
1006+
break
1007+
}
9861008
}
9871009
return nil
9881010
}

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

Lines changed: 71 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -520,43 +520,57 @@ module example.com
520520
-- a/a.go --
521521
package a
522522
523-
func f() {
523+
func f(x int) int {
524524
println("hello")
525525
defer println("world")
526+
return x
526527
}
527528
528529
func g() {
529530
println("goodbye")
530531
}
532+
533+
var v = [...]int{
534+
f(123),
535+
f(456),
536+
}
537+
538+
func init() {
539+
f(789)
540+
}
531541
`
532542
Run(t, files, func(t *testing.T, env *Env) {
533543
env.OpenFile("a/a.go")
534544

535-
// Invoke the "Browse assembly" code action to start the server.
536-
loc := env.RegexpSearch("a/a.go", "println")
537-
actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger)
538-
if err != nil {
539-
t.Fatalf("CodeAction: %v", err)
540-
}
541-
action, err := codeActionByKind(actions, settings.GoAssembly)
542-
if err != nil {
543-
t.Fatal(err)
544-
}
545+
asmFor := func(pattern string) []byte {
546+
// Invoke the "Browse assembly" code action to start the server.
547+
loc := env.RegexpSearch("a/a.go", pattern)
548+
actions, err := env.Editor.CodeAction(env.Ctx, loc, nil, protocol.CodeActionUnknownTrigger)
549+
if err != nil {
550+
t.Fatalf("CodeAction: %v", err)
551+
}
552+
action, err := codeActionByKind(actions, settings.GoAssembly)
553+
if err != nil {
554+
t.Fatal(err)
555+
}
545556

546-
// Execute the command.
547-
// Its side effect should be a single showDocument request.
548-
params := &protocol.ExecuteCommandParams{
549-
Command: action.Command.Command,
550-
Arguments: action.Command.Arguments,
551-
}
552-
var result command.DebuggingResult
553-
collectDocs := env.Awaiter.ListenToShownDocuments()
554-
env.ExecuteCommand(params, &result)
555-
doc := shownDocument(t, collectDocs(), "http:")
556-
if doc == nil {
557-
t.Fatalf("no showDocument call had 'file:' prefix")
557+
// Execute the command.
558+
// Its side effect should be a single showDocument request.
559+
params := &protocol.ExecuteCommandParams{
560+
Command: action.Command.Command,
561+
Arguments: action.Command.Arguments,
562+
}
563+
var result command.DebuggingResult
564+
collectDocs := env.Awaiter.ListenToShownDocuments()
565+
env.ExecuteCommand(params, &result)
566+
doc := shownDocument(t, collectDocs(), "http:")
567+
if doc == nil {
568+
t.Fatalf("no showDocument call had 'file:' prefix")
569+
}
570+
t.Log("showDocument(package doc) URL:", doc.URI)
571+
572+
return get(t, doc.URI)
558573
}
559-
t.Log("showDocument(package doc) URL:", doc.URI)
560574

561575
// Get the report and do some minimal checks for sensible results.
562576
//
@@ -567,23 +581,42 @@ func g() {
567581
// (e.g. uses JAL for CALL, or BL<cc> for RET).
568582
// We conservatively test only on the two most popular
569583
// architectures.
570-
report := get(t, doc.URI)
571-
checkMatch(t, true, report, `TEXT.*example.com/a.f`)
572-
switch runtime.GOARCH {
573-
case "amd64", "arm64":
574-
checkMatch(t, true, report, `CALL runtime.printlock`)
575-
checkMatch(t, true, report, `CALL runtime.printstring`)
576-
checkMatch(t, true, report, `CALL runtime.printunlock`)
577-
checkMatch(t, true, report, `CALL example.com/a.f.deferwrap1`)
578-
checkMatch(t, true, report, `RET`)
579-
checkMatch(t, true, report, `CALL runtime.morestack_noctxt`)
584+
{
585+
report := asmFor("println")
586+
checkMatch(t, true, report, `TEXT.*example.com/a.f`)
587+
switch runtime.GOARCH {
588+
case "amd64", "arm64":
589+
checkMatch(t, true, report, `CALL runtime.printlock`)
590+
checkMatch(t, true, report, `CALL runtime.printstring`)
591+
checkMatch(t, true, report, `CALL runtime.printunlock`)
592+
checkMatch(t, true, report, `CALL example.com/a.f.deferwrap1`)
593+
checkMatch(t, true, report, `RET`)
594+
checkMatch(t, true, report, `CALL runtime.morestack_noctxt`)
595+
}
596+
597+
// Nested functions are also shown.
598+
checkMatch(t, true, report, `TEXT.*example.com/a.f.deferwrap1`)
599+
600+
// But other functions are not.
601+
checkMatch(t, false, report, `TEXT.*example.com/a.g`)
580602
}
581603

582-
// Nested functions are also shown.
583-
checkMatch(t, true, report, `TEXT.*example.com/a.f.deferwrap1`)
604+
// Check that code in a package-level var initializer is found too.
605+
{
606+
report := asmFor(`f\(123\)`)
607+
checkMatch(t, true, report, `TEXT.*example.com/a.init`)
608+
checkMatch(t, true, report, `MOV. \$123`)
609+
checkMatch(t, true, report, `MOV. \$456`)
610+
checkMatch(t, true, report, `CALL example.com/a.f`)
611+
}
584612

585-
// But other functions are not.
586-
checkMatch(t, false, report, `TEXT.*example.com/a.g`)
613+
// And code in a source-level init function.
614+
{
615+
report := asmFor(`f\(789\)`)
616+
checkMatch(t, true, report, `TEXT.*example.com/a.init`)
617+
checkMatch(t, true, report, `MOV. \$789`)
618+
checkMatch(t, true, report, `CALL example.com/a.f`)
619+
}
587620
})
588621
}
589622

0 commit comments

Comments
 (0)