Skip to content

Commit

Permalink
fix: Error when missing reduce (#194)
Browse files Browse the repository at this point in the history
This puts in place additional layers of check to try and prevent wierd
situations arising from the use of `for`.
  • Loading branch information
DavePearce authored Jun 12, 2024
1 parent 5862e28 commit 5d4386a
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 5 deletions.
4 changes: 4 additions & 0 deletions src/compiler/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,10 @@ impl Intrinsic {
Ok(Node::from_expr(self.raw_call(args)))
}

pub fn unchecked_call(self, args: &[Node]) -> Result<Node> {
Ok(Node::from_expr(self.raw_call(args)))
}

pub fn raw_call(self, args: &[Node]) -> Expression {
Expression::Funcall {
func: self,
Expand Down
23 changes: 18 additions & 5 deletions src/compiler/generator.rs
Original file line number Diff line number Diff line change
Expand Up @@ -263,9 +263,17 @@ impl FuncVerifier<Node> for Intrinsic {
Intrinsic::Add
| Intrinsic::Sub
| Intrinsic::Mul
| Intrinsic::Neg
| Intrinsic::Normalize
| Intrinsic::VectorAdd
| Intrinsic::VectorSub
| Intrinsic::VectorMul => {}
| Intrinsic::VectorMul => {
for (i, arg) in args.iter().enumerate() {
if arg.is_list() {
bail!("unexpected list operand for {}", self.to_string())
}
}
}
Intrinsic::Begin => {
// TODO: maybe?
// if args_t
Expand Down Expand Up @@ -1316,9 +1324,9 @@ fn apply_form(
| Expression::Funcall { .. }
| Expression::Const(_) => panic!(),
Expression::List(xs) => {
if xs.is_empty() {
if xs.is_empty() {
Ok(Some(body))
} else if xs.len() == 1 {
} else if xs.len() == 1 {
Ok(Some(xs[0].clone()))
} else {
let mut r = apply_function(
Expand Down Expand Up @@ -1703,6 +1711,11 @@ pub(crate) fn reduce_toplevel(
let body = if let Some(guard) = guard {
let guard_expr = reduce(guard, &mut ctx, settings)?
.with_context(|| anyhow!("guard `{:?}` is empty", guard))?;
// Sanity check guard does not do strange things.
if !guard_expr.is_atomic() {
bail!("unexpected non-atomic guard in {}", handle.pretty())
}
// Sanity check guard has the expected type.
match guard_expr.t().c() {
Conditioning::Loobean => {
bail!("unexpected loobean guard in {}", handle.pretty())
Expand All @@ -1722,12 +1735,12 @@ pub(crate) fn reduce_toplevel(
// controlled exceptions to the usual loobean typing rules
let body_type = body.t();
Intrinsic::Mul
.call(&[persp_guard, body])?
.unchecked_call(&[persp_guard, body])
.with_context(|| anyhow!("constraint {}", name))?
.with_type(body_type)
} else {
body
};

if body.t() == Type::Void {
warn!(
"constraint {} should be of type {}, found {}",
Expand Down
44 changes: 44 additions & 0 deletions src/compiler/node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,50 @@ impl Node {
pub fn is_exocolumn(&self) -> bool {
matches!(self.e(), Expression::ExoColumn { .. })
}
/// Determine whether this expression is a list or not.
pub fn is_list(&self) -> bool {
match self.e() {
// Obvious fails
Expression::List(_) => true,
Expression::Funcall { func, .. } => match func {
Intrinsic::Begin => true,
_ => false,
},
_ => false,
}
}
/// Determines whether this expression is "atomic" or not. An
/// atomic expression is one which is never split into two or more
/// expressions. For example, an expression containing an
/// if-then-else conditional is not atomic, as it will split into
/// (at least) two expressions. Likewise, an expression
/// containing a list is not considered atomic.
pub fn is_atomic(&self) -> bool {
match self.e() {
// Obvious fails
Expression::List(_) => false,
Expression::Void => false,
// Obvious passes
Expression::Const(_) => true,
Expression::Column { .. }
| Expression::ExoColumn { .. }
| Expression::ArrayColumn { .. } => true,
// Other
Expression::Funcall { func, args } => match func {
Intrinsic::Begin => false,
Intrinsic::IfZero if args.len() > 2 => false,
Intrinsic::IfNotZero if args.len() > 2 => false,
_ => {
for arg in args {
if !arg.is_atomic() {
return false;
}
}
true
}
},
}
}
pub fn e(&self) -> &Expression {
&self._e
}
Expand Down

0 comments on commit 5d4386a

Please sign in to comment.