Restructure the "checking" pass in typestate

I noticed that typestate was being lazier than it should be,
because it was only checking typestate for statements and
top-level expression (that is, the expression in a stmt_expr, but
not any subexpressions). So I rewrote the checks in tstate/ck.rs
to use walk, which exposed a few bugs in typestate that I fixed.

Also added some more test cases for if-check.

Author: catamorphism

src/comp/middle/tstate/ann.rs

@@ -168,14 +168,22 @@ fn extend_poststate(&poststate p, &poststate new) -> bool {
     ret bitv::union(p, new);
 }
 
-
 // Clears the given bit in p
 fn relax_prestate(uint i, &prestate p) -> bool {
     auto was_set = bitv::get(p, i);
     bitv::set(p, i, false);
     ret was_set;
 }
 
+// Clears the given bit in p
+fn relax_poststate(uint i, &poststate p) -> bool {
+    ret relax_prestate(i, p);
+}
+
+// Clears the given bit in p
+fn relax_precond(uint i, &precond p) {
+    relax_prestate(i, p);
+}
 
 // Clears all the bits in p
 fn clear(&precond p) { bitv::clear(p); }

src/comp/middle/tstate/auxiliary.rs

@@ -358,10 +358,18 @@ fn stmt_poststate(&crate_ctxt ccx, &stmt s) -> poststate {
     ret stmt_states(ccx, s).poststate;
 }
 
+fn block_precond(&crate_ctxt ccx, &block b) -> precond {
+    ret block_pp(ccx, b).precondition;
+}
+
 fn block_postcond(&crate_ctxt ccx, &block b) -> postcond {
     ret block_pp(ccx, b).postcondition;
 }
 
+fn block_prestate(&crate_ctxt ccx, &block b) -> prestate {
+    ret block_states(ccx, b).prestate;
+}
+
 fn block_poststate(&crate_ctxt ccx, &block b) -> poststate {
     ret block_states(ccx, b).poststate;
 }
@@ -402,11 +410,15 @@ fn set_pre_and_post(&crate_ctxt ccx, &ann a, &precond pre, &postcond post) {
 fn copy_pre_post(&crate_ctxt ccx, &ann a, &@expr sub) {
     log "set_pre_and_post";
     auto p = expr_pp(ccx, sub);
-    auto t = ann_to_ts_ann(ccx, a);
-    set_precondition(t, p.precondition);
-    set_postcondition(t, p.postcondition);
+    copy_pre_post_(ccx, a, p.precondition, p.postcondition);
 }
 
+fn copy_pre_post_(&crate_ctxt ccx, &ann a, &prestate pre, &poststate post) {
+    log "set_pre_and_post";
+    auto t = ann_to_ts_ann(ccx, a);
+    set_precondition(t, pre);
+    set_postcondition(t, post);
+}
 
 /* sets all bits to *1* */
 fn set_postcond_false(&crate_ctxt ccx, &ann a) {
@@ -548,7 +560,9 @@ fn expr_to_constr_arg(ty::ctxt tcx, &@expr e) -> @constr_arg_use {
         }
         case (expr_lit(?l, _)) { ret @respan(e.span, carg_lit(l)); }
         case (_) {
-            tcx.sess.bug("exprs_to_constr_args: ill-formed pred arg");
+            tcx.sess.span_err(e.span,
+                              "Arguments to constrained functions must be "
+                              + "literals or local variables");
         }
     }
 }
@@ -626,6 +640,11 @@ fn path_to_ident(&ty::ctxt cx, &path p) -> ident {
         case (some(?i)) { ret i; }
     }
 }
+
+tag if_ty { 
+    if_check;
+    plain_if;
+}
 //
 // Local Variables:
 // mode: rust

src/comp/middle/tstate/bitvectors.rs

@@ -15,13 +15,21 @@ import aux::npred;
 import aux::pred_desc;
 import aux::match_args;
 import aux::constr_;
+import aux::block_precond;
+import aux::stmt_precond;
+import aux::expr_precond;
+import aux::block_prestate;
+import aux::expr_prestate;
+import aux::stmt_prestate;
 import tstate::aux::ann_to_ts_ann;
 import tstate::ann::pre_and_post;
 import tstate::ann::precond;
 import tstate::ann::postcond;
 import tstate::ann::prestate;
 import tstate::ann::poststate;
 import tstate::ann::relax_prestate;
+import tstate::ann::relax_precond;
+import tstate::ann::relax_poststate;
 import tstate::ann::pps_len;
 import tstate::ann::true_precond;
 import tstate::ann::empty_prestate;
@@ -136,9 +144,46 @@ fn gen(&fn_ctxt fcx, &ann a, &constr_ c) -> bool {
 fn declare_var(&fn_ctxt fcx, &constr_ c, prestate pre) -> prestate {
     auto res = clone(pre);
     relax_prestate(bit_num(fcx, c), res);
+    // idea is this is scoped
+    relax_poststate(bit_num(fcx, c), res);
     ret res;
 }
 
+fn relax_precond_block_non_recursive(&fn_ctxt fcx, uint i, &block b) {
+    relax_precond(i, block_precond(fcx.ccx, b));
+}
+
+fn relax_precond_expr(&fn_ctxt fcx, uint i, &@expr e) {
+    relax_precond(i, expr_precond(fcx.ccx, e));
+}
+
+fn relax_precond_stmt(&fn_ctxt fcx, uint i, &@stmt s) {
+    relax_precond(i, stmt_precond(fcx.ccx, *s));
+}
+
+fn relax_precond_block(&fn_ctxt fcx, uint i, &block b) {
+    relax_precond_block_non_recursive(fcx, i, b);
+    // FIXME: should use visit instead
+    // could at least generalize this pattern 
+    // (also seen in ck::check_states_against_conditions)
+    let @mutable bool keepgoing = @mutable true;
+
+    fn quit(@mutable bool keepgoing, &@item i) {
+        *keepgoing = false;
+    }
+    fn kg(@mutable bool keepgoing) -> bool { ret *keepgoing; }
+
+    auto v = rec(visit_block_pre = bind
+                    relax_precond_block_non_recursive(fcx, i, _),
+                 visit_expr_pre  = bind relax_precond_expr(fcx, i, _),
+                 visit_stmt_pre  = bind relax_precond_stmt(fcx, i, _),
+                  visit_item_pre=bind quit(keepgoing, _),
+                  keep_going=bind kg(keepgoing)
+
+                   with walk::default_visitor());
+    walk::walk_block(v, b);
+}
+
 fn gen_poststate(&fn_ctxt fcx, &ann a, &constr_ c) -> bool {
     log "gen_poststate";
     ret set_in_poststate(bit_num(fcx, c), ann_to_ts_ann(fcx.ccx, a).states);

src/comp/middle/tstate/ck.rs

@@ -61,9 +61,19 @@ import collect_locals::mk_f_to_fn_info;
 import pre_post_conditions::fn_pre_post;
 import states::find_pre_post_state_fn;
 
-fn check_states_expr(&fn_ctxt fcx, @expr e) {
+fn check_states_expr(&fn_ctxt fcx, &@expr e) {
     let precond prec = expr_precond(fcx.ccx, e);
     let prestate pres = expr_prestate(fcx.ccx, e);
+
+    /*
+    log_err("check_states_expr:");
+      util::common::log_expr_err(*e);
+      log_err("prec = ");
+      log_bitv_err(fcx, prec);
+      log_err("pres = ");
+      log_bitv_err(fcx, pres);
+    */
+
     if (!implies(pres, prec)) {
         auto s = "";
         auto diff = first_difference_string(fcx, prec, pres);
@@ -79,26 +89,27 @@ fn check_states_expr(&fn_ctxt fcx, @expr e) {
     }
 }
 
-fn check_states_stmt(&fn_ctxt fcx, &stmt s) {
-    auto a = stmt_to_ann(fcx.ccx, s);
+fn check_states_stmt(&fn_ctxt fcx, &@stmt s) {
+    auto a = stmt_to_ann(fcx.ccx, *s);
     let precond prec = ann_precond(a);
     let prestate pres = ann_prestate(a);
 
-    /*    
+    /*
       log_err("check_states_stmt:");
-      log_stmt_err(s);
+      log_stmt_err(*s);
       log_err("prec = ");
-      log_bitv_err(fcx.enclosing, prec);
+      log_bitv_err(fcx, prec);
       log_err("pres = ");
-      log_bitv_err(fcx.enclosing, pres);
+      log_bitv_err(fcx, pres);
     */
+
     if (!implies(pres, prec)) {
         auto ss = "";
         auto diff = first_difference_string(fcx, prec, pres);
         ss +=
             "Unsatisfied precondition constraint (for example, " + diff +
                 ") for statement:\n";
-        ss += pretty::pprust::stmt_to_str(s);
+        ss += pretty::pprust::stmt_to_str(*s);
         ss += "\nPrecondition:\n";
         ss += bitv_to_str(fcx, prec);
         ss += "\nPrestate: \n";
@@ -107,42 +118,50 @@ fn check_states_stmt(&fn_ctxt fcx, &stmt s) {
     }
 }
 
-fn check_states_against_conditions(&fn_ctxt fcx, &_fn f, &ann a) {
-    auto enclosing = fcx.enclosing;
-    auto nv = num_constraints(enclosing);
-    auto post = @mutable empty_poststate(nv);
-    fn do_one_(fn_ctxt fcx, &@stmt s, @mutable poststate post) {
-        check_states_stmt(fcx, *s);
-        *post = stmt_poststate(fcx.ccx, *s);
+fn check_states_against_conditions(&fn_ctxt fcx, &_fn f, &ann a,
+                                   &span sp, &ident i, &def_id d) {
+    /* Postorder traversal instead of pre is important
+       because we want the smallest possible erroneous statement
+       or expression. */
+
+    let @mutable bool keepgoing = @mutable true;
+    
+    /* TODO probably should use visit instead */
+
+    fn quit(@mutable bool keepgoing, &@ast::item i) {
+        *keepgoing = false;
     }
-    auto do_one = bind do_one_(fcx, _, post);
-    vec::map[@stmt, ()](do_one, f.body.node.stmts);
-    fn do_inner_(fn_ctxt fcx, &@expr e, @mutable poststate post) {
-        check_states_expr(fcx, e);
-        *post = expr_poststate(fcx.ccx, e);
+    fn kg(@mutable bool keepgoing) -> bool { 
+        ret *keepgoing;
     }
-    auto do_inner = bind do_inner_(fcx, _, post);
-    option::map[@expr, ()](do_inner, f.body.node.expr);
-    auto cf = fcx.enclosing.cf;
-    /* Finally, check that the return value is initialized */
 
+    auto v = rec (visit_stmt_post=bind check_states_stmt(fcx, _),
+                  visit_expr_post=bind check_states_expr(fcx, _),
+                  visit_item_pre=bind quit(keepgoing, _),
+                  keep_going=bind kg(keepgoing)
+                  with walk::default_visitor());
+
+    walk::walk_fn(v, f, sp, i, d, a);
+
+    /* Finally, check that the return value is initialized */
+    auto post = aux::block_poststate(fcx.ccx, f.body);
     let aux::constr_ ret_c = rec(id=fcx.id, c=aux::ninit(fcx.name));
-    if (f.proto == ast::proto_fn && !promises(fcx, { *post }, ret_c) &&
+    if (f.proto == ast::proto_fn && !promises(fcx, post, ret_c) &&
             !type_is_nil(fcx.ccx.tcx, ret_ty_of_fn(fcx.ccx.tcx, a)) &&
-            cf == return) {
+            f.decl.cf == return) {
         fcx.ccx.tcx.sess.span_note(f.body.span,
                                    "In function " + fcx.name +
                                        ", not all control paths \
                                         return a value");
         fcx.ccx.tcx.sess.span_err(f.decl.output.span,
                                   "see declared return type of '" +
                                       ty_to_str(*f.decl.output) + "'");
-    } else if (cf == noreturn) {
+    } else if (f.decl.cf == noreturn) {
 
         // check that this really always fails
         // the fcx.id bit means "returns" for a returning fn,
         // "diverges" for a non-returning fn
-        if (!promises(fcx, { *post }, ret_c)) {
+        if (!promises(fcx, post, ret_c)) {
             fcx.ccx.tcx.sess.span_err(f.body.span,
                                       "In non-returning function " + fcx.name
                                           +
@@ -152,15 +171,16 @@ fn check_states_against_conditions(&fn_ctxt fcx, &_fn f, &ann a) {
     }
 }
 
-fn check_fn_states(&fn_ctxt fcx, &_fn f, &ann a) {
+fn check_fn_states(&fn_ctxt fcx, &_fn f, &ann a, &span sp, &ident i,
+                   &def_id d) {
     /* Compute the pre- and post-states for this function */
 
     auto g = find_pre_post_state_fn;
     fixed_point_states(fcx, g, f);
     /* Now compare each expr's pre-state to its precondition
        and post-state to its postcondition */
 
-    check_states_against_conditions(fcx, f, a);
+    check_states_against_conditions(fcx, f, a, sp, i, d);
 }
 
 fn fn_states(&crate_ctxt ccx, &_fn f, &span sp, &ident i, &def_id id,
@@ -170,7 +190,7 @@ fn fn_states(&crate_ctxt ccx, &_fn f, &span sp, &ident i, &def_id id,
     assert (ccx.fm.contains_key(id));
     auto f_info = ccx.fm.get(id);
     auto fcx = rec(enclosing=f_info, id=id, name=i, ccx=ccx);
-    check_fn_states(fcx, f, a);
+    check_fn_states(fcx, f, a, sp, i, id);
 }
 
 fn check_crate(ty::ctxt cx, @crate crate) {
@@ -185,15 +205,15 @@ fn check_crate(ty::ctxt cx, @crate crate) {
 
     auto do_pre_post = walk::default_visitor();
     do_pre_post =
-        rec(visit_fn_pre=bind fn_pre_post(ccx, _, _, _, _, _)
+        rec(visit_fn_post=bind fn_pre_post(ccx, _, _, _, _, _)
             with do_pre_post);
     walk::walk_crate(do_pre_post, *crate);
     /* Check the pre- and postcondition against the pre- and poststate
        for every expression */
 
     auto do_states = walk::default_visitor();
     do_states =
-        rec(visit_fn_pre=bind fn_states(ccx, _, _, _, _, _) with do_states);
+        rec(visit_fn_post=bind fn_states(ccx, _, _, _, _, _) with do_states);
     walk::walk_crate(do_states, *crate);
 }
 //