Skip to content

Commit

Permalink
improve variadic funcs matching rules (#29)
Browse files Browse the repository at this point in the history
Introduced a new matcher op: maybeVariadicCallExpr.
Unlike CallExpr, it doesn't unconditionally accept
both variadic and non-variadic calls. Instead, it only
accepts variadic calls if there are more than N arguments.

Take this pattern for example:

	f($_, $_, $_, $*_)

Variadic call will be accepted only if there are more than
3 arguments passed to f.

	f(a, b, c, 4)     // ok
	f(a, b, c, xs...) // ok
	f(a, b, xs...)    // NOT ok

Fixes #28
  • Loading branch information
quasilyte authored Mar 20, 2022
1 parent 628d8b3 commit f7f5e21
Show file tree
Hide file tree
Showing 7 changed files with 226 additions and 151 deletions.
32 changes: 24 additions & 8 deletions compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -467,25 +467,41 @@ func (c *compiler) compileExprMembers(list []ast.Expr) {
}

func (c *compiler) compileCallExpr(n *ast.CallExpr) {
canBeVariadic := func(n *ast.CallExpr) bool {
variadicOperation := func(n *ast.CallExpr) (operation, uint8) {
if len(n.Args) == 0 {
return false
return opNonVariadicCallExpr, 0
}
lastArg, ok := n.Args[len(n.Args)-1].(*ast.Ident)
if !ok {
return false
return opNonVariadicCallExpr, 0
}
if !isWildName(lastArg.Name) || !decodeWildName(lastArg.Name).Seq {
return opNonVariadicCallExpr, 0
}
if len(n.Args) == 1 {
return opCallExpr, 0
}
// If there is any seq op before the lastArg, we emit opCallExpr too.
for i := 0; i < len(n.Args)-1; i++ {
if decodeWildNode(n.Args[i]).Seq {
return opCallExpr, 0
}
}
return isWildName(lastArg.Name) && decodeWildName(lastArg.Name).Seq
return opMaybeVariadicCallExpr, c.toUint8(n, len(n.Args)-1)
}

op := opNonVariadicCallExpr
var value uint8
var op operation
if n.Ellipsis.IsValid() {
op = opVariadicCallExpr
} else if canBeVariadic(n) {
op = opCallExpr
} else {
op, value = variadicOperation(n)
}

c.emitInstOp(op)
c.prog.insts = append(c.prog.insts, instruction{
op: op,
value: value,
})
c.compileSymbol(n.Fun)
c.compileExprMembers(n.Args)
}
Expand Down
22 changes: 21 additions & 1 deletion compile_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -331,14 +331,25 @@ func TestCompileWildcard(t *testing.T) {
},

`f(1, $*_)`: {
`CallExpr`,
`MaybeVariadicCallExpr 1`,
` • Ident f`,
` • ArgList`,
` • • BasicLit 1`,
` • • NodeSeq`,
` • • End`,
},

`f(1, 2, 3, $*_)`: {
`MaybeVariadicCallExpr 3`,
` • Ident f`,
` • ArgList`,
` • • BasicLit 1`,
` • • BasicLit 2`,
` • • BasicLit 3`,
` • • NodeSeq`,
` • • End`,
},

`f($_)`: {
`NonVariadicCallExpr`,
` • Ident f`,
Expand Down Expand Up @@ -453,6 +464,15 @@ func TestCompileWildcard(t *testing.T) {
` • NamedNode v`,
` • NamedNode xs`,
},

`f($_, $*_)`: {
`MaybeVariadicCallExpr 1`,
` • Ident f`,
` • ArgList`,
` • • Node`,
` • • NodeSeq`,
` • • End`,
},
})

for i := range tests {
Expand Down
1 change: 1 addition & 0 deletions gen_operations.go
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ var opPrototypes = []operationProto{

{name: "VariadicCallExpr", tag: "CallExpr", args: "fn args", example: "f(1, xs...)"},
{name: "NonVariadicCallExpr", tag: "CallExpr", args: "fn args", example: "f(1, xs)"},
{name: "MaybeVariadicCallExpr", tag: "CallExpr", args: "fn args", value: "int | can be variadic if len(args)>value", example: "f(1, xs) or f(1, xs...)"},
{name: "CallExpr", tag: "CallExpr", args: "fn args", example: "f(1, xs) or f(1, xs...)"},

{name: "AssignStmt", tag: "AssignStmt", args: "lhs rhs", value: "token.Token | ':=' or '='", example: "lhs := rhs()"},
Expand Down
9 changes: 9 additions & 0 deletions match.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,15 @@ func (m *matcher) matchNodeWithInst(state *MatcherState, inst instruction, n ast
case opNonVariadicCallExpr:
n, ok := n.(*ast.CallExpr)
return ok && !n.Ellipsis.IsValid() && m.matchNode(state, n.Fun) && m.matchArgList(state, n.Args)
case opMaybeVariadicCallExpr:
n, ok := n.(*ast.CallExpr)
if !ok {
return false
}
if n.Ellipsis.IsValid() && len(n.Args) <= int(inst.value) {
return false
}
return m.matchNode(state, n.Fun) && m.matchArgList(state, n.Args)
case opCallExpr:
n, ok := n.(*ast.CallExpr)
return ok && m.matchNode(state, n.Fun) && m.matchArgList(state, n.Args)
Expand Down
14 changes: 14 additions & 0 deletions match_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -432,6 +432,20 @@ func TestMatch(t *testing.T) {
{`fmt.Sprintf($_, $args)`, 0, `fmt.Sprintf(f, a...)`},
{`$fmt.$_($*_)`, 1, `fmt.Sprintf("%d", 1)`},

// OK: trailing $*_ can match variadic calls.
{`f($*_)`, 1, `f(xs...)`},
{`f($*_)`, 1, `f(1, xs...)`},
{`f($_, $*_)`, 1, `f(1, xs...)`},
{`f($_, $*_)`, 1, `f(1, 2, xs...)`},
{`f($_, $_, $*_)`, 1, `f(1, 2, xs...)`},
{`f($_, $_, $*_)`, 1, `f(1, 2, 3, xs...)`},
{`f($*_, $_, $*_)`, 1, `f(1)`},
// TODO: the case below should probably be rejected.
{`f($*_, $_, $*_)`, 1, `f(xs...)`},
// It doesn't allow to match a variadic call in the following cases.
{`f($_, $*_)`, 0, `f(xs...)`},
{`f($_, $_, $*_)`, 0, `f(1, xs...)`},

// Selector expr.
{`$x.Field`, 1, `a.Field`},
{`$x.Field`, 1, `b.Field`},
Expand Down
145 changes: 73 additions & 72 deletions operation_string.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading

0 comments on commit f7f5e21

Please sign in to comment.