Skip to content

Commit

Permalink
interp: fix the detection of builtin calls during parsing
Browse files Browse the repository at this point in the history
Avoid shortcuts, and apply regular scoping rules for symbol
resolution when checking for builtin calls.

Fixes #1173.
  • Loading branch information
mvertes authored Jul 8, 2021
1 parent 297b40d commit 25b570d
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 6 deletions.
10 changes: 10 additions & 0 deletions _test/issue-1173.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package main

var real = func() { println("Hello") }

func main() {
real()
}

// Output:
// Hello
5 changes: 2 additions & 3 deletions interp/cfg.go
Original file line number Diff line number Diff line change
Expand Up @@ -864,7 +864,7 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
case callExpr:
wireChild(n)
switch {
case interp.isBuiltinCall(n):
case isBuiltinCall(n, sc):
c0 := n.child[0]
bname := c0.ident
err = check.builtin(bname, n, n.child[1:], n.action == aCallSlice)
Expand Down Expand Up @@ -1260,11 +1260,10 @@ func (interp *Interpreter) cfg(root *node, importPath string) ([]*node, error) {
}
}
// Found symbol, populate node info
n.typ, n.findex, n.level = sym.typ, sym.index, level
n.sym, n.typ, n.findex, n.level = sym, sym.typ, sym.index, level
if n.findex < 0 {
n.val = sym.node
} else {
n.sym = sym
switch {
case sym.kind == constSym && sym.rval.IsValid():
n.rval = sym.rval
Expand Down
11 changes: 8 additions & 3 deletions interp/type.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ func nodeType(interp *Interpreter, sc *scope, n *node) (*itype, error) {
t = dt

case callExpr:
if interp.isBuiltinCall(n) {
if isBuiltinCall(n, sc) {
// Builtin types are special and may depend from their input arguments.
t.cat = builtinT
switch n.child[0].ident {
Expand Down Expand Up @@ -675,11 +675,16 @@ func nodeType(interp *Interpreter, sc *scope, n *node) (*itype, error) {
return t, err
}

func (interp *Interpreter) isBuiltinCall(n *node) bool {
func isBuiltinCall(n *node, sc *scope) bool {
if n.kind != callExpr {
return false
}
s := interp.universe.sym[n.child[0].ident]
s := n.child[0].sym
if s == nil {
if sym, _, found := sc.lookup(n.child[0].ident); found {
s = sym
}
}
return s != nil && s.kind == bltnSym
}

Expand Down

3 comments on commit 25b570d

@TheMightyGit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvertes This commit introduced an error in my app (reflect: call of reflect.Value.Call on ptr Value) - I'm trying to work out a minimal repro (it's deep in the app somewhere) and will raise an issue when I work it out.

@TheMightyGit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mvertes I'm struggling to make a small repro for the new error I get starting with this commit. I'll describe the problem in case that helps. I have a struct func that creates a closure that calls a global func - but when that closure is called later on (as part of an event driven system) it panics. The only fix I've found is to make a local copy of the global func inside the struct func and use that in the closure instead of the global func. Oddly this fixes it, it's like the closure somehow loses the global scope and panics when called, but it retains the scope of the struct func where it was created so using a local copy of the global func makes it work again.

I'm really struggling to make a small repro for this. I've coded what I've said above but it doesn't exhibit the same problem, so it may be something in my more complex app that's breaking the global scope somehow. All I can say is it worked before this commit.

I'll keep puzzling over it until I can get a small repro.

@TheMightyGit
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I finally found a (not simple) repro for this. Reported here: #1206

Please sign in to comment.