handle numeric fields in struct syntax

tjhance · tjhance · commit 3a20c67fe882 · 2023-12-27T14:23:05.000-05:00
diff --git a/source/rust_verify/src/lifetime_ast.rs b/source/rust_verify/src/lifetime_ast.rs
@@ -11,6 +11,7 @@ pub(crate) enum IdKind {
     Fun,
     Local,
     Builtin,
+    Field,
 }
 
 #[derive(Debug, Clone, PartialEq, Eq, Hash)]
diff --git a/source/rust_verify/src/lifetime_emit.rs b/source/rust_verify/src/lifetime_emit.rs
@@ -12,6 +12,8 @@ pub(crate) fn encode_id(kind: IdKind, rename_count: usize, raw_id: &String) -> S
         IdKind::Fun => format!("f{}_{}", rename_count, raw_id),
         IdKind::Local => format!("x{}_{}", rename_count, vir::def::user_local_name(raw_id)),
         IdKind::Builtin => raw_id.clone(),
+        IdKind::Field if raw_id.bytes().nth(0).unwrap().is_ascii_digit() => raw_id.clone(),
+        IdKind::Field => format!("y{}_{}", rename_count, vir::def::user_local_name(raw_id)),
     }
 }
 
diff --git a/source/rust_verify/src/lifetime_generate.rs b/source/rust_verify/src/lifetime_generate.rs
@@ -3,7 +3,7 @@ use crate::erase::{ErasureHints, ResolvedCall};
 use crate::rust_to_vir_base::{
     def_id_to_vir_path, local_to_var, mid_ty_const_to_vir, mid_ty_to_vir_datatype,
 };
-use crate::rust_to_vir_expr::get_adt_res;
+use crate::rust_to_vir_expr::{field_name_to_vir_ident, get_adt_res};
 use crate::verus_items::{PervasiveItem, RustItem, VerusItem, VerusItems};
 use crate::{lifetime_ast::*, verus_items};
 use air::ast_util::str_ident;
@@ -77,6 +77,7 @@ pub(crate) struct State {
     datatype_worklist: Vec<DefId>,
     imported_fun_worklist: Vec<DefId>,
     id_to_name: HashMap<String, Id>,
+    field_to_name: HashMap<String, Id>,
     typ_param_to_name: HashMap<(String, Option<u32>), Id>,
     lifetime_to_name: HashMap<(String, Option<u32>), Id>,
     fun_to_name: HashMap<Fun, Id>,
@@ -99,6 +100,7 @@ impl State {
             datatype_worklist: Vec::new(),
             imported_fun_worklist: Vec::new(),
             id_to_name: HashMap::new(),
+            field_to_name: HashMap::new(),
             typ_param_to_name: HashMap::new(),
             lifetime_to_name: HashMap::new(),
             fun_to_name: HashMap::new(),
@@ -137,6 +139,12 @@ impl State {
         Self::id(&mut self.rename_count, &mut self.id_to_name, IdKind::Local, &raw_id, f)
     }
 
+    fn field<S: Into<String>>(&mut self, raw_id: S) -> Id {
+        let raw_id = raw_id.into();
+        let f = || raw_id.clone();
+        Self::id(&mut self.rename_count, &mut self.field_to_name, IdKind::Field, &raw_id, f)
+    }
+
     fn typ_param<S: Into<String>>(&mut self, raw_id: S, maybe_impl_index: Option<u32>) -> Id {
         let raw_id = raw_id.into();
         let (is_impl, impl_index) = match (raw_id.starts_with("impl "), maybe_impl_index) {
@@ -495,7 +503,7 @@ fn erase_pat<'tcx>(ctxt: &Context<'tcx>, state: &mut State, pat: &Pat) -> Patter
             let name = state.datatype_name(&vir_path);
             let mut binders: Vec<(Id, Pattern)> = Vec::new();
             for pat in pats.iter() {
-                let field = state.local(pat.ident.to_string());
+                let field = state.field(pat.ident.to_string());
                 let pattern = erase_pat(ctxt, state, &pat.pat);
                 binders.push((field, pattern));
             }
@@ -1114,9 +1122,9 @@ fn erase_expr<'tcx>(
                 let variant = datatype.x.get_variant(&variant_name);
                 let mut fs: Vec<(Id, Exp)> = Vec::new();
                 for f in fields.iter() {
-                    let field_name = Arc::new(f.ident.as_str().to_string());
-                    let (_, field_mode, _) = get_field(&variant.a, &field_name).a;
-                    let name = state.local(f.ident.to_string());
+                    let vir_field_name = field_name_to_vir_ident(f.ident.as_str());
+                    let (_, field_mode, _) = get_field(&variant.a, &vir_field_name).a;
+                    let name = state.field(f.ident.to_string());
                     let e = if field_mode == Mode::Spec {
                         phantom_data_expr(ctxt, state, &f.expr)
                     } else {
@@ -1204,12 +1212,7 @@ fn erase_expr<'tcx>(
             if expect_spec {
                 erase_spec_exps(ctxt, state, expr, vec![exp1])
             } else {
-                let field_name = field.to_string();
-                let field_id = if field_name.chars().all(char::is_numeric) {
-                    Id::new(IdKind::Builtin, 0, field_name)
-                } else {
-                    state.local(field.to_string())
-                };
+                let field_id = state.field(field.to_string());
                 mk_exp(ExpX::Field(exp1.expect("expr"), field_id))
             }
         }
@@ -2067,7 +2070,7 @@ fn erase_variant_data<'tcx>(
         None => {
             let mut fields: Vec<Field> = Vec::new();
             for field in &variant.fields {
-                let name = state.local(field.ident(ctxt.tcx).to_string());
+                let name = state.field(field.ident(ctxt.tcx).to_string());
                 let typ = erase_ty(ctxt, state, &ctxt.tcx.type_of(field.did).skip_binder());
                 fields.push(Field { name, typ: revise_typ(field.did, typ) });
             }
diff --git a/source/rust_verify/src/rust_to_vir_expr.rs b/source/rust_verify/src/rust_to_vir_expr.rs
@@ -40,7 +40,7 @@ use vir::ast::{
     Typ, TypX, UnaryOp, UnaryOpr, VirErr,
 };
 use vir::ast_util::{ident_binder, typ_to_diagnostic_str, types_equal, undecorate_typ};
-use vir::def::positional_field_ident;
+use vir::def::{positional_field_ident, positional_field_ident_for_str};
 
 pub(crate) fn pat_to_mut_var<'tcx>(pat: &Pat) -> Result<(bool, String), VirErr> {
     let Pat { hir_id: _, kind, span, default_binding_modes } = pat;
@@ -481,7 +481,8 @@ pub(crate) fn pattern_to_vir_inner<'tcx>(
             let mut binders: Vec<Binder<vir::ast::Pattern>> = Vec::new();
             for fpat in pats.iter() {
                 let pattern = pattern_to_vir(bctx, &fpat.pat)?;
-                let binder = ident_binder(&str_ident(&fpat.ident.as_str()), &pattern);
+                let ident = field_name_to_vir_ident(fpat.ident.as_str());
+                let binder = ident_binder(&ident, &pattern);
                 binders.push(binder);
             }
             PatternX::Constructor(vir_path, variant_name, Arc::new(binders))
@@ -1650,7 +1651,8 @@ pub(crate) fn expr_to_vir_innermost<'tcx>(
                     .iter()
                     .map(|f| -> Result<_, VirErr> {
                         let vir = expr_to_vir(bctx, f.expr, modifier)?;
-                        Ok(ident_binder(&str_ident(&f.ident.as_str()), &vir))
+                        let ident = field_name_to_vir_ident(f.ident.as_str());
+                        Ok(ident_binder(&ident, &vir))
                     })
                     .collect::<Result<Vec<_>, _>>()?,
             );
@@ -2201,3 +2203,11 @@ pub(crate) fn closure_to_vir<'tcx>(
         panic!("closure_to_vir expects ExprKind::Closure");
     }
 }
+
+pub(crate) fn field_name_to_vir_ident(name: &str) -> vir::ast::Ident {
+    if name.bytes().nth(0).unwrap().is_ascii_digit() {
+        positional_field_ident_for_str(name)
+    } else {
+        str_ident(name)
+    }
+}
diff --git a/source/rust_verify_test/tests/adts.rs b/source/rust_verify_test/tests/adts.rs
@@ -1088,3 +1088,82 @@ test_verify_one_file! {
         }
     } => Ok(())
 }
+
+test_verify_one_file! {
+    #[test] struct_syntax_with_numeric_field_names verus_code! {
+        #[is_variant]
+        enum Foo {
+            Bar(u32, u32),
+            Qux,
+        }
+
+        fn test() {
+            let b = Foo::Bar { 1: 30, 0: 20 };
+            assert(b.get_Bar_0() == 20);
+            assert(b.get_Bar_1() == 30);
+        }
+
+        fn test2() {
+            let b = Foo::Bar { 1: 30, 0: 20 };
+            assert(b.get_Bar_1() == 20); // FAILS
+        }
+
+
+        fn test_pat(foo: Foo) {
+            let foo = Foo::Bar(20, 40);
+            match foo {
+                Foo::Bar { 1: a, 0: b } => {
+                    assert(b == 20);
+                    assert(a == 40);
+                }
+                Foo::Qux => { assert(false); }
+            }
+        }
+
+        fn test_pat2(foo: Foo) {
+            let foo = Foo::Bar(20, 40);
+            match foo {
+                Foo::Bar { 1: a, 0: b } => {
+                    assert(b == 40); // FAILS
+                }
+                Foo::Qux => { }
+            }
+        }
+
+        fn test_pat_not_all_fields(foo: Foo) {
+            let foo = Foo::Bar(20, 40);
+            match foo {
+                Foo::Bar { 1: a, .. } => {
+                    assert(a == 40);
+                }
+                Foo::Qux => { assert(false); }
+            }
+        }
+
+        fn test_pat_not_all_fields2(foo: Foo) {
+            let foo = Foo::Bar(20, 40);
+            match foo {
+                Foo::Bar { 1: a, .. } => {
+                    assert(a == 20); // FAILS
+                }
+                Foo::Qux => { }
+            }
+        }
+
+        spec fn sfn(foo: Foo) -> (u32, u32) {
+            match foo {
+                Foo::Bar { 1: a, 0: b } => (b, a),
+                Foo::Qux => (0, 0),
+            }
+        }
+
+        proof fn test_sfn(foo: Foo) {
+            assert(sfn(Foo::Bar(20, 30)) == (20u32, 30u32));
+            assert(sfn(Foo::Qux) == (0u32, 0u32));
+        }
+
+        proof fn test_sfn2(foo: Foo) {
+            assert(sfn(Foo::Bar(20, 30)).0 == 30); // FAILS
+        }
+    } => Err(err) => assert_fails(err, 4)
+}
diff --git a/source/rust_verify_test/tests/modes.rs b/source/rust_verify_test/tests/modes.rs
@@ -950,9 +950,8 @@ test_verify_one_file! {
 }
 
 test_verify_one_file! {
-    // TODO(utaal) issue with tracked rewrite, I believe
-    #[ignore] #[test] test_struct_pattern_fields_out_of_order_fail_issue_348 verus_code! {
-        struct Foo {
+    #[test] test_struct_pattern_fields_out_of_order_fail_issue_348 verus_code! {
+        tracked struct Foo {
             ghost a: u64,
             tracked b: u64,
         }
@@ -992,6 +991,24 @@ test_verify_one_file! {
     } => Ok(())
 }
 
+test_verify_one_file! {
+    #[test] test_struct_pattern_fields_numeric_out_of_order_fail verus_code! {
+        tracked struct Foo(ghost u64, tracked u64);
+
+        proof fn some_call(tracked y: u64) { }
+
+        proof fn t() {
+            let tracked foo = Foo(5, 6);
+            let tracked Foo { 1: b, 0: a } = foo;
+
+            // Variable 'a' has the mode of field '0' (that is, spec)
+            // some_call requires 'proof'
+            // So this should fail
+            some_call(a);
+        }
+    } => Err(err) => assert_vir_error_msg(err, "expression has mode spec, expected mode proof")
+}
+
 test_verify_one_file! {
     #[test] test_use_exec_var_in_forall verus_code! {
         spec fn some_fn(j: int) -> bool {
diff --git a/source/vir/src/ast.rs b/source/vir/src/ast.rs
@@ -480,8 +480,8 @@ pub enum PatternX {
     /// Note: ast_simplify replaces this with Constructor
     Tuple(Patterns),
     /// Match constructor of datatype Path, variant Ident
-    /// For tuple-style variants, the patterns appear in order and are named "0", "1", etc.
-    /// For struct-style variants, the patterns may appear in any order.
+    /// For tuple-style variants, the fields are named "_0", "_1", etc.
+    /// Fields can appear **in any order** even for tuple variants.
     Constructor(Path, Ident, Binders<Pattern>),
     Or(Pattern, Pattern),
 }
@@ -611,8 +611,8 @@ pub enum ExprX {
     Tuple(Exprs),
     /// Construct datatype value of type Path and variant Ident,
     /// with field initializers Binders<Expr> and an optional ".." update expression.
-    /// For tuple-style variants, the field initializers appear in order and are named "_0", "_1", etc.
-    /// For struct-style variants, the field initializers may appear in any order.
+    /// For tuple-style variants, the fields are named "_0", "_1", etc.
+    /// Fields can appear **in any order** even for tuple variants.
     Ctor(Path, Ident, Binders<Expr>, Option<Expr>),
     /// Primitive 0-argument operation
     NullaryOpr(NullaryOpr),
diff --git a/source/vir/src/def.rs b/source/vir/src/def.rs
@@ -474,6 +474,10 @@ pub fn positional_field_ident(idx: usize) -> Ident {
     Arc::new(format!("_{}", idx))
 }
 
+pub fn positional_field_ident_for_str(idx: &str) -> Ident {
+    Arc::new(format!("_{}", idx))
+}
+
 pub fn monotyp_apply(datatype: &Path, args: &Vec<Path>) -> Path {
     if args.len() == 0 {
         datatype.clone()

Original file line number	Diff line number	Diff line change
`@@ -11,6 +11,7 @@ pub(crate) enum IdKind {`
`11`	`11`	`Fun,`
`12`	`12`	`Local,`
`13`	`13`	`Builtin,`
	`14`	`+ Field,`
`14`	`15`	`}`
`15`	`16`
`16`	`17`	`#[derive(Debug, Clone, PartialEq, Eq, Hash)]`
Original file line number	Diff line number	Diff line change
`@@ -12,6 +12,8 @@ pub(crate) fn encode_id(kind: IdKind, rename_count: usize, raw_id: &String) -> S`
`12`	`12`	`IdKind::Fun => format!("f{}_{}", rename_count, raw_id),`
`13`	`13`	`IdKind::Local => format!("x{}_{}", rename_count, vir::def::user_local_name(raw_id)),`
`14`	`14`	`IdKind::Builtin => raw_id.clone(),`
	`15`	`+ IdKind::Field if raw_id.bytes().nth(0).unwrap().is_ascii_digit() => raw_id.clone(),`
	`16`	`+ IdKind::Field => format!("y{}_{}", rename_count, vir::def::user_local_name(raw_id)),`
`15`	`17`	`}`
`16`	`18`	`}`
`17`	`19`