Skip to content

Commit

Permalink
Fix semantic check for function references
Browse files Browse the repository at this point in the history
These changes fix an issue in the semantic checks against function
references. The compiler is supposed to catch references to functions
that have not been applied (e.g., f(1) is OK but just f is not.) The
compiler was failing to catch these references when they were embedded
within composites. If these references were encountered during
evaluation, it resulted in undefined behaviour.

The fix for this issue is to simply remove the flag that gated whether
terms were checked. The flag was in place to deal with nested terms like
f(g(x)) but that has not been applicable for a while since these terms
are rewritten (and have been since the unifier was reimplemented over
year ago.)

Fixes open-policy-agent#1132

Signed-off-by: Torin Sandall <torinsandall@gmail.com>
  • Loading branch information
tsandall committed Jan 4, 2019
1 parent c4e6349 commit f3df4dc
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 11 deletions.
15 changes: 5 additions & 10 deletions ast/check.go
Original file line number Diff line number Diff line change
Expand Up @@ -472,9 +472,8 @@ func (tc *typeChecker) err(err *Error) {
}

type refChecker struct {
env *TypeEnv
errs Errors
checkTerm bool
env *TypeEnv
errs Errors
}

func newRefChecker(env *TypeEnv) *refChecker {
Expand All @@ -496,17 +495,13 @@ func (rc *refChecker) Visit(x interface{}) Visitor {
}
return nil
case *Term:
rc.checkTerm = true
Walk(rc, terms)
rc.checkTerm = false
return nil
}
case Ref:
if rc.checkTerm {
if err := rc.checkApply(rc.env, x); err != nil {
rc.errs = append(rc.errs, err)
return nil
}
if err := rc.checkApply(rc.env, x); err != nil {
rc.errs = append(rc.errs, err)
return nil
}
if err := rc.checkRef(rc.env, rc.env.tree, x, 0); err != nil {
rc.errs = append(rc.errs, err)
Expand Down
13 changes: 12 additions & 1 deletion ast/check_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -623,7 +623,6 @@ func TestCheckBuiltinErrors(t *testing.T) {
{"objects-bad-input", `sum({"a": 1, "b": 2}, x)`},
{"sets-any", `sum({1,2,"3",4}, x)`},
{"virtual-ref", `plus(data.test.p, data.deabeef, 0)`},
{"function-ref", `data.test.f(1, data.test.f)`},
}

env := newTestEnv([]string{
Expand Down Expand Up @@ -901,6 +900,18 @@ func TestUserFunctionsTypeInference(t *testing.T) {
`f(x) = y { {"k": x} = y }`,
false,
},
{
`p { [data.base.foo] }`,
true,
},
{
`p { x = data.base.foo }`,
true,
},
{
`p { data.base.foo(data.base.bar) }`,
true,
},
}

for n, test := range tests {
Expand Down

0 comments on commit f3df4dc

Please sign in to comment.