Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

handle numeric fields in struct syntax #949

Merged
merged 3 commits into from
Jan 14, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions source/rust_verify/src/lifetime_ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ pub(crate) enum IdKind {
Fun,
Local,
Builtin,
Field,
}

#[derive(Debug, Clone, PartialEq, Eq, Hash)]
Expand Down
2 changes: 2 additions & 0 deletions source/rust_verify/src/lifetime_emit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)),
}
}

Expand Down
27 changes: 15 additions & 12 deletions source/rust_verify/src/lifetime_generate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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>,
Expand All @@ -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(),
Expand Down Expand Up @@ -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)
Copy link
Contributor

@utaal-b utaal-b Jan 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will this store the name in key_to_name even for numeric fields for which we are not going to use the rename_count in lifetime_emit? Does that matter?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is the distinction in lifetime_emit necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess it's not necessary, now that I think about it. I guess the original reason I did it was because I didn't think it made sense to use IdKind::Local for field names.

}

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) {
Expand Down Expand Up @@ -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));
}
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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))
}
}
Expand Down Expand Up @@ -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) });
}
Expand Down
16 changes: 13 additions & 3 deletions source/rust_verify/src/rust_to_vir_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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))
Expand Down Expand Up @@ -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<_>, _>>()?,
);
Expand Down Expand Up @@ -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)
}
}
79 changes: 79 additions & 0 deletions source/rust_verify_test/tests/adts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}
23 changes: 20 additions & 3 deletions source/rust_verify_test/tests/modes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
Expand Down Expand Up @@ -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 {
Expand Down
8 changes: 4 additions & 4 deletions source/vir/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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),
}
Expand Down Expand Up @@ -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),
Expand Down
4 changes: 4 additions & 0 deletions source/vir/src/def.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
Loading