Skip to content

Commit

Permalink
Fix "undefined function" error with mutually-recursive functions (#184)
Browse files Browse the repository at this point in the history
Fix "undefined function" error with mutually-recursive functions

When the AWK code defines mutually-recursive functions, the topological
sort can't help us. So define all funcInfos in the initial call-graph
pass before doing any resolver passes. This avoids the "undefined
function" error in those cases.

Add a minimal test for this, and also add a test of parsing/using the
gron.awk code, as it's a nice, fairly complex test case.

Fixes #183. Thanks @xonixx for the bug report!
  • Loading branch information
benhoyt authored Jun 5, 2023
1 parent 230d423 commit 92e0799
Show file tree
Hide file tree
Showing 5 changed files with 422 additions and 41 deletions.
2 changes: 1 addition & 1 deletion goawk.go
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ import (
)

const (
version = "v1.23.1"
version = "v1.23.2"
copyright = "GoAWK " + version + " - Copyright (c) 2022 Ben Hoyt"
shortUsage = "usage: goawk [-F fs] [-v var=value] [-f progfile | 'prog'] [file ...]"
longUsage = `Standard AWK arguments:
Expand Down
4 changes: 4 additions & 0 deletions goawk_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -449,6 +449,10 @@ testdata/g.6:2/6: Duo
"", "", "testdata/parseerror/bad.awk:2:3: expected expression instead of <newline>\nx*\n ^"},
{[]string{"-f", "testdata/parseerror/good.awk", "-f", "-", "-f", "testdata/parseerror/bad.awk"},
"`", "", "<stdin>:1:1: unexpected char\n`\n^"},

// Other programs
{[]string{"-f", "testdata/other/gron.awk"}, `{"foo":42}`,
"json={}\njson.foo=42\n", ""},
}
for _, test := range tests {
testName := strings.Join(test.args, " ")
Expand Down
75 changes: 35 additions & 40 deletions internal/resolver/resolve.go
Original file line number Diff line number Diff line change
Expand Up @@ -125,12 +125,25 @@ func Resolve(prog *ast.Program, config *Config) *ResolvedProgram {
config = &Config{}
}

// Assign indexes to native (Go-defined) functions, in order of name.
// Do this before our first pass, so that AWK-defined functions override
// Go-defined ones and take precedence.
funcInfo := make(map[string]FuncInfo)
var nativeNames []string
for name := range config.Funcs {
nativeNames = append(nativeNames, name)
}
sort.Strings(nativeNames)
for i, name := range nativeNames {
funcInfo[name] = FuncInfo{Native: true, Index: i}
}

// First pass determines call graph so we can process functions in
// topological order: e.g., if f() calls g(), process g first, then f.
callGraph := callGraphVisitor{
calls: make(map[string]map[string]struct{}),
funcs: make(map[string]*ast.Function),
funcIndexes: make(map[string]int),
calls: make(map[string]map[string]struct{}),
funcs: make(map[string]*ast.Function),
funcInfo: funcInfo,
}
ast.Walk(&callGraph, prog)
orderedFuncs := topoSort(callGraph.calls)
Expand All @@ -147,33 +160,31 @@ func Resolve(prog *ast.Program, config *Config) *ResolvedProgram {
}
}

// Create our type resolver.
r := resolver{
varInfo: make(map[string]map[string]VarInfo),
funcInfo: make(map[string]FuncInfo),
funcs: callGraph.funcs,
// Define the local variable names (we don't know their types yet).
varInfo := make(map[string]map[string]VarInfo)
for funcName, info := range funcInfo {
if info.Native {
continue
}
varInfo[funcName] = make(map[string]VarInfo)
for _, param := range info.Params {
varInfo[funcName][param] = VarInfo{}
}
}

// Create our type resolver.
r := resolver{varInfo: varInfo, funcInfo: funcInfo, funcs: callGraph.funcs}
r.varInfo[""] = make(map[string]VarInfo) // func of "" stores global vars

// Interpreter relies on ARGV and other built-in arrays being present.
r.recordVar("", "ARGV", Array, lexer.Position{1, 1})
r.recordVar("", "ENVIRON", Array, lexer.Position{1, 1})
r.recordVar("", "FIELDS", Array, lexer.Position{1, 1})

// Assign indexes to native (Go-defined) functions, in order of name.
var nativeNames []string
for name := range config.Funcs {
nativeNames = append(nativeNames, name)
}
sort.Strings(nativeNames)
for i, name := range nativeNames {
r.funcInfo[name] = FuncInfo{Native: true, Index: i}
}

// Main resolver pass: determine types of variables and find function
// information. Can't call ast.Walk on prog directly, as it will not
// iterate through functions in topological (call graph) order.
main := mainVisitor{r: &r, nativeFuncs: config.Funcs, funcIndexes: callGraph.funcIndexes}
main := mainVisitor{r: &r, nativeFuncs: config.Funcs}
main.walkOrdered(prog, orderedFuncs)

// Do another pass to set parameter types in functions which don't use
Expand Down Expand Up @@ -305,27 +316,13 @@ func (r *resolver) recordVar(funcName, varName string, typ Type, pos lexer.Posit
}
}

// Define a function and its local variables.
func (r *resolver) defineFunc(index int, name string, params []string) {
if info, ok := r.funcInfo[name]; ok && !info.Native {
// Function already defined in the previous pass
return
}
r.funcInfo[name] = FuncInfo{Native: false, Index: index, Params: params}
// Define the local variable names (we don't know their types yet).
r.varInfo[name] = make(map[string]VarInfo)
for _, param := range params {
r.varInfo[name][param] = VarInfo{}
}
}

// callGraphVisitor records what functions are called by the current function
// to build our call graph.
type callGraphVisitor struct {
calls map[string]map[string]struct{} // map of current function to called function
funcs map[string]*ast.Function
funcIndexes map[string]int
curFunc string
calls map[string]map[string]struct{} // map of current function to called function
funcs map[string]*ast.Function
funcInfo map[string]FuncInfo
curFunc string
}

func (v *callGraphVisitor) Visit(node ast.Node) ast.Visitor {
Expand All @@ -334,8 +331,8 @@ func (v *callGraphVisitor) Visit(node ast.Node) ast.Visitor {
if _, ok := v.funcs[n.Name]; ok {
panic(ast.PosErrorf(n.Pos, "function %q already defined", n.Name))
}
v.funcInfo[n.Name] = FuncInfo{Index: len(v.funcs), Params: n.Params}
v.funcs[n.Name] = n
v.funcIndexes[n.Name] = len(v.funcIndexes)
v.curFunc = n.Name
ast.WalkStmtList(v, n.Body)
v.curFunc = ""
Expand All @@ -357,7 +354,6 @@ func (v *callGraphVisitor) Visit(node ast.Node) ast.Visitor {
type mainVisitor struct {
r *resolver
nativeFuncs map[string]interface{}
funcIndexes map[string]int
curFunc string
}

Expand All @@ -374,7 +370,6 @@ func (v *mainVisitor) walkOrdered(prog *ast.Program, orderedFuncs []string) {
// and flagged as an error in the visitor.
continue
}
v.r.defineFunc(v.funcIndexes[funcName], funcName, function.Params)
v.curFunc = funcName
ast.WalkStmtList(v, function.Body)
v.curFunc = ""
Expand Down
5 changes: 5 additions & 0 deletions interp/interp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -743,6 +743,11 @@ BEGIN { foo(5); bar(10) }
{`function f(x) { print x, x(); } BEGIN { f() }`, "", "", `parse error at 1:26: can't call local variable "x" as function`, "function"},
{`function f(x) { print x } BEGIN { print f(1, 2) } # !gawk`, // only a warning in Gawk
"", "", `parse error at 1:41: "f" called with more arguments than declared`, ""},
{`# mutually recursive functions
function f(n) { if (!n) return; print "f(" n ")"; g(n-1) }
function g(n) { if (!n) return; print "g(" n ")"; f(n-1) }
BEGIN { f(4) }
`, "", "f(4)\ng(3)\nf(2)\ng(1)\n", "", ""},

// Redirected I/O
{`BEGIN { getline x; print x }`, "foo", "foo\n", "", ""},
Expand Down
Loading

0 comments on commit 92e0799

Please sign in to comment.