Skip to content

Commit

Permalink
[Fix] Properly force enum variants (#1835)
Browse files Browse the repository at this point in the history
* Properly force enum variants

The implementation of deep_seq and force primops haven't been properly
updated to handle enum variants, letting them mostly unevaluated. This
could in particular let contracts be unexpectedly unevaluated, and also
show unevaluated chunk in the REPL when inputting enum variants there,
which isn't ideal.

This commit properly forces enum variants.

* Apply suggestions from code review

Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>

---------

Co-authored-by: Viktor Kleen <viktor.kleen@tweag.io>
  • Loading branch information
yannham and vkleen authored Mar 5, 2024
1 parent d823d7f commit 72f4a37
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 0 deletions.
1 change: 1 addition & 0 deletions core/src/eval/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,7 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
.map(|result| result.body)
}

/// Same as [Self::eval_full], but takes a closure as an argument instead of a term.
pub fn eval_full_closure(&mut self, t0: Closure) -> Result<Closure, EvalError> {
self.eval_deep_closure_impl(t0, false)
.map(|result| Closure {
Expand Down
21 changes: 21 additions & 0 deletions core/src/eval/operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -707,6 +707,10 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
env: Environment::new(),
})
}
Term::EnumVariant { arg, .. } => Ok(Closure {
body: seq_terms(std::iter::once(arg), pos_op),
env,
}),
_ => {
if let Some((next, ..)) = self.stack.pop_arg(&self.cache) {
Ok(next)
Expand Down Expand Up @@ -1063,6 +1067,23 @@ impl<R: ImportResolver, C: Cache> VirtualMachine<R, C> {
env: Environment::new(),
})
}
// For an enum variant, `force x` is simply equivalent to `deep_seq x x`, as
// there's no lazy pending contract to apply.
Term::EnumVariant { tag, arg, attrs } => {
let cont = RichTerm::new(
Term::EnumVariant {
tag,
arg: arg.clone(),
attrs,
},
pos.into_inherited(),
);

Ok(Closure {
body: seq_terms(std::iter::once(arg), pos_op, cont),
env,
})
}
_ => Ok(Closure {
body: RichTerm { term: t, pos },
env
Expand Down
7 changes: 7 additions & 0 deletions core/tests/integration/inputs/adts/deep_seq_enum.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# test.type = 'error'
#
# [test.metadata]
# error = 'EvalError::BlameError'

# Check that deep_seq correctly evaluates the content of an enum variant
%deep_seq% ('Foo 5 | [| 'Foo String |]) null
7 changes: 7 additions & 0 deletions core/tests/integration/inputs/adts/force_enum.ncl
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# test.type = 'error'
#
# [test.metadata]
# error = 'EvalError::BlameError'

# Check that force correctly evaluates the content of an enum variant
%force% ('Foo 5 | [| 'Foo String |])

0 comments on commit 72f4a37

Please sign in to comment.