Skip to content

Commit

Permalink
cmd/compile: copy literals when inlining
Browse files Browse the repository at this point in the history
Without this, literals keep their original source positions through
inlining, which results in strange jumps in line numbers of inlined
function bodies. By copying literals, inlining can update their source
position like other nodes.

Fixes golang#15453.

Change-Id: Iad5d9bbfe183883794213266dc30e31bab89ee69
Reviewed-on: https://go-review.googlesource.com/37232
Run-TryBot: David Lazar <[email protected]>
TryBot-Result: Gobot Gobot <[email protected]>
Reviewed-by: Matthew Dempsky <[email protected]>
Reviewed-by: Russ Cox <[email protected]>
  • Loading branch information
davidlazar committed Mar 3, 2017
1 parent 699175a commit 1c6ef9a
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 16 deletions.
43 changes: 27 additions & 16 deletions src/cmd/compile/internal/gc/inl.go
Original file line number Diff line number Diff line change
Expand Up @@ -977,7 +977,12 @@ func (subst *inlsubst) node(n *Node) *Node {
return n

case OLITERAL, OTYPE:
return n
// If n is a named constant or type, we can continue
// using it in the inline copy. Otherwise, make a copy
// so we can update the line number.
if n.Sym != nil {
return n
}

// Since we don't handle bodies with closures, this return is guaranteed to belong to the current inlined function.

Expand Down Expand Up @@ -1015,24 +1020,24 @@ func (subst *inlsubst) node(n *Node) *Node {
m.Left = newname(lookup(p))

return m
default:
m := nod(OXXX, nil, nil)
*m = *n
m.Ninit.Set(nil)

if n.Op == OCLOSURE {
Fatalf("cannot inline function containing closure: %+v", n)
}
}

m.Left = subst.node(n.Left)
m.Right = subst.node(n.Right)
m.List.Set(subst.list(n.List))
m.Rlist.Set(subst.list(n.Rlist))
m.Ninit.Set(append(m.Ninit.Slice(), subst.list(n.Ninit)...))
m.Nbody.Set(subst.list(n.Nbody))
m := nod(OXXX, nil, nil)
*m = *n
m.Ninit.Set(nil)

return m
if n.Op == OCLOSURE {
Fatalf("cannot inline function containing closure: %+v", n)
}

m.Left = subst.node(n.Left)
m.Right = subst.node(n.Right)
m.List.Set(subst.list(n.List))
m.Rlist.Set(subst.list(n.Rlist))
m.Ninit.Set(append(m.Ninit.Slice(), subst.list(n.Ninit)...))
m.Nbody.Set(subst.list(n.Nbody))

return m
}

// setPos is a visitor to update position info with a new inlining index.
Expand All @@ -1051,6 +1056,12 @@ func (s *setPos) node(n *Node) {
if n == nil {
return
}
if n.Op == OLITERAL || n.Op == OTYPE {
if n.Sym != nil {
// This node is not a copy, so don't clobber position.
return
}
}

// don't clobber names, unless they're freshly synthesized
if n.Op != ONAME || !n.Pos.IsKnown() {
Expand Down
50 changes: 50 additions & 0 deletions test/inline_literal.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
// run

// Copyright 2017 The Go Authors. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file.

package main

import (
"log"
"reflect"
"runtime"
)

func hello() string {
return "Hello World" // line 16
}

func foo() string { // line 19
x := hello() // line 20
y := hello() // line 21
return x + y // line 22
}

func bar() string {
x := hello() // line 26
return x
}

// funcPC returns the PC for the func value f.
func funcPC(f interface{}) uintptr {
return reflect.ValueOf(f).Pointer()
}

// Test for issue #15453. Previously, line 26 would appear in foo().
func main() {
pc := funcPC(foo)
f := runtime.FuncForPC(pc)
for ; runtime.FuncForPC(pc) == f; pc++ {
file, line := f.FileLine(pc)
if line == 0 {
continue
}
// Line 16 can appear inside foo() because PC-line table has
// innermost line numbers after inlining.
if line != 16 && !(line >= 19 && line <= 22) {
log.Fatalf("unexpected line at PC=%d: %s:%d\n", pc, file, line)
}
}
}

0 comments on commit 1c6ef9a

Please sign in to comment.