From f39477d9260c5b346df5e5a71abaaca891a8195d Mon Sep 17 00:00:00 2001
From: Tim Chevalier <chevalier@alum.wellesley.edu>
Date: Tue, 10 Jul 2012 18:24:41 -0700
Subject: [PATCH] In resolve, check that an or-pattern has the same number of
 bindings in each disjunct

resolve3 wasn't checking this. Added test cases. Also added a helpful informational
message in the case where you have a variable binding that you probably think
refers to a variant that you forgot to import.

This is easier to do in resolve than in typeck because there's code in typeck
that assumes that each of the patterns binds the same number of variables.
---
 src/rustc/middle/pat_util.rs        |  3 --
 src/rustc/middle/resolve3.rs        | 44 +++++++++++++++++++++++++++--
 src/test/compile-fail/issue-2848.rs | 15 ++++++++++
 src/test/compile-fail/issue-2849.rs |  7 +++++
 4 files changed, 64 insertions(+), 5 deletions(-)
 create mode 100644 src/test/compile-fail/issue-2848.rs
 create mode 100644 src/test/compile-fail/issue-2849.rs

diff --git a/src/rustc/middle/pat_util.rs b/src/rustc/middle/pat_util.rs
index 16be926d7be1..482f22cc6abe 100644
--- a/src/rustc/middle/pat_util.rs
+++ b/src/rustc/middle/pat_util.rs
@@ -34,9 +34,6 @@ fn pat_is_variant(dm: resolve::def_map, pat: @pat) -> bool {
     }
 }
 
-// This does *not* normalize. The pattern should be already normalized
-// if you want to get a normalized pattern out of it.
-// Could return a constrained type in order to express that (future work)
 fn pat_bindings(dm: resolve::def_map, pat: @pat,
                 it: fn(node_id, span, @path)) {
     do walk_pat(pat) |p| {
diff --git a/src/rustc/middle/resolve3.rs b/src/rustc/middle/resolve3.rs
index 3ea829fe9ef6..d3502fb96441 100644
--- a/src/rustc/middle/resolve3.rs
+++ b/src/rustc/middle/resolve3.rs
@@ -17,8 +17,9 @@ import syntax::ast::{ident, trait_ref, impure_fn, instance_var, item};
 import syntax::ast::{item_class, item_const, item_enum, item_fn, item_mac};
 import syntax::ast::{item_foreign_mod, item_trait, item_impl, item_mod};
 import syntax::ast::{item_ty, local, local_crate, method, node_id, pat};
-import syntax::ast::{pat_enum, pat_ident, pat_lit, pat_range, path, prim_ty};
-import syntax::ast::{stmt_decl, ty};
+import syntax::ast::{pat_enum, pat_ident, path, prim_ty, stmt_decl, ty,
+                     pat_box, pat_uniq, pat_lit, pat_range, pat_rec,
+                     pat_tup, pat_wild};
 import syntax::ast::{ty_bool, ty_char, ty_constr, ty_f, ty_f32, ty_f64};
 import syntax::ast::{ty_float, ty_i, ty_i16, ty_i32, ty_i64, ty_i8, ty_int};
 import syntax::ast::{ty_param, ty_path, ty_str, ty_u, ty_u16, ty_u32, ty_u64};
@@ -28,6 +29,7 @@ import syntax::ast::{view_path_list, view_path_simple};
 import syntax::ast_util::{def_id_of_def, dummy_sp, local_def, new_def_hash};
 import syntax::ast_util::{walk_pat};
 import syntax::attr::{attr_metas, contains_name};
+import syntax::print::pprust::path_to_str;
 import syntax::codemap::span;
 import syntax::visit::{default_visitor, fk_method, mk_vt, visit_block};
 import syntax::visit::{visit_crate, visit_expr, visit_expr_opt, visit_fn};
@@ -3384,6 +3386,40 @@ class Resolver {
                              none, visitor);
     }
 
+    fn num_bindings(pat: @pat) -> uint {
+      pat_util::pat_binding_ids(self.def_map, pat).len()
+    }
+
+    fn warn_var_patterns(arm: arm) {
+      /*
+        The idea here is that an arm like:
+           alpha | beta
+        where alpha is a variant and beta is an identifier that
+        might refer to a variant that's not in scope will result
+        in a confusing error message. Showing that beta actually binds a
+        new variable might help.
+       */
+      for arm.pats.each |p| {
+         do pat_util::pat_bindings(self.def_map, p) |_id, sp, pth| {
+             self.session.span_note(sp, #fmt("Treating %s as a variable \
+               binding, because it does not denote any variant in scope",
+                                             path_to_str(pth)));
+         }
+      };
+    }
+    fn check_consistent_bindings(arm: arm) {
+      if arm.pats.len() == 0 { ret; }
+      let good = self.num_bindings(arm.pats[0]);
+      for arm.pats.each() |p: @pat| {
+        if self.num_bindings(p) != good {
+          self.session.span_err(p.span,
+             "inconsistent number of bindings");
+          self.warn_var_patterns(arm);
+          break;
+        };
+      };
+    }
+
     fn resolve_arm(arm: arm, visitor: ResolveVisitor) {
         (*self.value_ribs).push(@Rib(NormalRibKind));
 
@@ -3393,6 +3429,10 @@ class Resolver {
                                  some(bindings_list), visitor);
         }
 
+        // This has to happen *after* we determine which
+        // pat_idents are variants
+        self.check_consistent_bindings(arm);
+
         visit_expr_opt(arm.guard, (), visitor);
         self.resolve_block(arm.body, visitor);
 
diff --git a/src/test/compile-fail/issue-2848.rs b/src/test/compile-fail/issue-2848.rs
new file mode 100644
index 000000000000..606c5fc8e003
--- /dev/null
+++ b/src/test/compile-fail/issue-2848.rs
@@ -0,0 +1,15 @@
+mod bar {
+    enum foo {
+        alpha,
+        beta,
+        charlie
+    }
+}
+
+fn main() {
+    import bar::{alpha, charlie};
+    alt alpha {
+      alpha | beta {} //~ ERROR: inconsistent number of bindings
+      charlie {}
+    }
+}
diff --git a/src/test/compile-fail/issue-2849.rs b/src/test/compile-fail/issue-2849.rs
new file mode 100644
index 000000000000..5fed9f2d26c2
--- /dev/null
+++ b/src/test/compile-fail/issue-2849.rs
@@ -0,0 +1,7 @@
+enum foo { alpha, beta(int) }
+
+fn main() {
+    alt alpha {
+      alpha | beta(i) {} //~ ERROR inconsistent number of bindings
+    }
+}