diff --git a/third_party/move/move-compiler-v2/src/bytecode_generator.rs b/third_party/move/move-compiler-v2/src/bytecode_generator.rs index e020f1171297d..25fb126bd2b5a 100644 --- a/third_party/move/move-compiler-v2/src/bytecode_generator.rs +++ b/third_party/move/move-compiler-v2/src/bytecode_generator.rs @@ -139,6 +139,11 @@ impl<'env> Generator<'env> { self.func_env.module_env.env } + /// Shortcut to get type of a node. + fn get_node_type(&'env self, id: NodeId) -> Type { + self.env().get_node_type(id) + } + /// Emit a bytecode. fn emit(&mut self, b: Bytecode) { self.code.push(b) @@ -197,7 +202,7 @@ impl<'env> Generator<'env> { self.error( id, format!("cannot assign tuple type `{}` to single variable (use `(a, b, ..) = ..` instead)", - ty.display(&self.env().get_type_display_ctx())) + ty.display(&self.env().get_type_display_ctx())) ) } let next_idx = self.temps.len(); @@ -299,7 +304,6 @@ impl<'env> Generator<'env> { // Result is thrown away, but for typing reasons, we need to introduce // temps to construct the step target. let step_targets = self - .env() .get_node_type(step.node_id()) .flatten() .into_iter() @@ -318,7 +322,7 @@ impl<'env> Generator<'env> { // Declare all variables bound by the pattern let mut scope = BTreeMap::new(); for (id, sym) in pat.vars() { - let ty = self.env().get_node_type(id); + let ty = self.get_node_type(id); let temp = self.new_temp_with_valid_type(id, ty); scope.insert(sym, temp); } @@ -430,7 +434,7 @@ impl<'env> Generator<'env> { /// Convert a value from AST world into a constant as expected in bytecode. fn to_constant(&self, id: NodeId, val: &Value) -> Constant { - let ty = self.env().get_node_type(id); + let ty = self.get_node_type(id); match val { Value::Address(x) => Constant::Address(x.clone()), Value::Number(x) => match ty { @@ -521,10 +525,25 @@ impl<'env> Generator<'env> { self.gen_op_call(targets, id, BytecodeOperation::Pack(*mid, *sid, inst), args) }, Operation::Select(mid, sid, fid) => { - let inst = self.env().get_node_instantiation(id); let target = self.require_unary_target(id, targets); let arg = self.require_unary_arg(id, args); - self.gen_select(target, id, mid.qualified_inst(*sid, inst), *fid, &arg) + // Get the instantiation of the struct. It is not contained in the select + // expression but in the type of it's operand. + if let Some((_, inst)) = self + .get_node_type(arg.node_id()) + .skip_reference() + .get_struct(self.env()) + { + self.gen_select( + target, + id, + mid.qualified_inst(*sid, inst.to_vec()), + *fid, + &arg, + ) + } else { + self.internal_error(id, "inconsistent type in select expression") + } }, Operation::Exists(None) => { let inst = self.env().get_node_instantiation(id); @@ -654,7 +673,7 @@ impl<'env> Generator<'env> { } fn gen_cast_call(&mut self, targets: Vec, id: NodeId, args: &[Exp]) { - let ty = self.env().get_node_type(id); + let ty = self.get_node_type(id); let bytecode_op = match ty { Type::Primitive(PrimitiveType::U8) => BytecodeOperation::CastU8, Type::Primitive(PrimitiveType::U16) => BytecodeOperation::CastU16, @@ -763,7 +782,7 @@ impl<'env> Generator<'env> { /// for `&mut` to `&` conversion, in which case we need to to introduce a Freeze operation. fn maybe_convert(&self, exp: &Exp, expected_ty: &Type) -> Exp { let id = exp.node_id(); - let exp_ty = self.env().get_node_type(id); + let exp_ty = self.get_node_type(id); if let ( Type::Reference(ReferenceKind::Mutable, _), Type::Reference(ReferenceKind::Immutable, et), @@ -788,14 +807,11 @@ impl<'env> Generator<'env> { match exp.as_ref() { ExpData::Temporary(_, temp) => *temp, ExpData::LocalVar(id, sym) => self.find_local(*id, *sym), - ExpData::Call(_, Operation::Select(..), _) if self.reference_mode() => { + ExpData::Call(id, Operation::Select(..), _) if self.reference_mode() => { // In reference mode, a selection is interpreted as selecting a reference to the // field. - let id = exp.node_id(); - let ty = Type::Reference( - self.reference_mode_kind, - Box::new(self.env().get_node_type(id)), - ); + let ty = + Type::Reference(self.reference_mode_kind, Box::new(self.get_node_type(*id))); let temp = self.new_temp(ty); self.gen(vec![temp], exp); temp @@ -803,7 +819,7 @@ impl<'env> Generator<'env> { _ => { // Otherwise, introduce a temporary let id = exp.node_id(); - let ty = self.env().get_node_type(id); + let ty = self.get_node_type(id); let temp = self.new_temp(ty); self.gen(vec![temp], exp); temp @@ -844,10 +860,10 @@ impl<'env> Generator<'env> { impl<'env> Generator<'env> { fn gen_borrow(&mut self, target: TempIndex, id: NodeId, kind: ReferenceKind, arg: &Exp) { match arg.as_ref() { - ExpData::Call(_arg_id, Operation::Select(mid, sid, fid), args) => { + ExpData::Call(arg_id, Operation::Select(mid, sid, fid), args) => { return self.gen_borrow_field( target, - id, + *arg_id, kind, mid.qualified(*sid), *fid, @@ -924,13 +940,27 @@ impl<'env> Generator<'env> { // Currently no conditions for immutable, so we do allow `&fun().field`. }, } - let inst = self.env().get_node_instantiation(id); - self.emit_call( - id, - vec![target], - BytecodeOperation::BorrowField(struct_id.module_id, struct_id.id, inst, field_offset), - vec![temp], - ); + // Get instantiation of field. It is not contained in the select expression but in the + // type of its operand. + if let Some((_, inst)) = self + .get_node_type(oper.node_id()) + .skip_reference() + .get_struct(self.env()) + { + self.emit_call( + id, + vec![target], + BytecodeOperation::BorrowField( + struct_id.module_id, + struct_id.id, + inst.to_vec(), + field_offset, + ), + vec![temp], + ); + } else { + self.internal_error(id, "inconsistent type in select expression") + } } } @@ -1100,7 +1130,7 @@ impl<'env> Generator<'env> { Pattern::Wildcard(id) => { // Wildcard pattern: we need to create a temporary to receive the value, even // if its dropped afterwards. - let temp = self.new_temp(self.env().get_node_type(*id)); + let temp = self.new_temp(self.get_node_type(*id)); (temp, None) }, Pattern::Var(id, sym) => { @@ -1112,7 +1142,7 @@ impl<'env> Generator<'env> { // Pattern is not flat: create a new temporary and an Assignment of this // temporary to the pattern. let id = pat.node_id(); - let ty = self.env().get_node_type(id); + let ty = self.get_node_type(id); let temp = self.new_temp_with_valid_type(id, ty); (temp, Some((id, pat.clone(), temp))) }, diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs b/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs index d36b00148961f..0361aba923f55 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/function_generator.rs @@ -84,7 +84,12 @@ struct LabelInfo { impl<'a> FunctionGenerator<'a> { /// Runs the function generator for the given function. - pub fn run<'b>(gen: &'a mut ModuleGenerator, ctx: &'b ModuleContext, fun_env: FunctionEnv<'b>) { + pub fn run<'b>( + gen: &'a mut ModuleGenerator, + ctx: &'b ModuleContext, + fun_env: FunctionEnv<'b>, + acquires_list: &BTreeSet, + ) { let loc = fun_env.get_loc(); let function = gen.function_index(ctx, &loc, &fun_env); let visibility = fun_env.visibility(); @@ -111,11 +116,18 @@ impl<'a> FunctionGenerator<'a> { } else { (gen, None) }; + let acquires_global_resources = acquires_list + .iter() + .map(|id| { + let struct_env = fun_env.module_env.get_struct(*id); + gen.struct_def_index(ctx, &struct_env.get_loc(), &struct_env) + }) + .collect(); let def = FF::FunctionDefinition { function, visibility, is_entry: fun_env.is_entry(), - acquires_global_resources: vec![], + acquires_global_resources, code, }; ctx.checked_bound( diff --git a/third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs b/third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs index a761f3ec26667..b3e7f1245d25c 100644 --- a/third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs +++ b/third_party/move/move-compiler-v2/src/file_format_generator/module_generator.rs @@ -27,10 +27,11 @@ use move_model::{ ty::{PrimitiveType, ReferenceKind, Type}, }; use move_stackless_bytecode::{ - function_target_pipeline::FunctionTargetsHolder, stackless_bytecode::Constant, + function_target_pipeline::{FunctionTargetsHolder, FunctionVariant}, + stackless_bytecode::{Bytecode, Constant, Operation}, }; use move_symbol_pool::symbol as IR_SYMBOL; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; /// Internal state of the module code generator #[derive(Debug)] @@ -144,8 +145,11 @@ impl ModuleGenerator { for struct_env in module_env.get_structs() { self.gen_struct(ctx, &struct_env) } + + let acquires_map = ctx.generate_acquires_map(module_env); for fun_env in module_env.get_functions() { - FunctionGenerator::run(self, ctx, fun_env); + let acquires_list = &acquires_map[&fun_env.get_id()]; + FunctionGenerator::run(self, ctx, fun_env, acquires_list); } } @@ -766,3 +770,61 @@ impl<'env> ModuleContext<'env> { s.display(self.env.symbol_pool()).to_string() } } + +impl<'env> ModuleContext<'env> { + /// Acquires analysis. This is temporary until we have the full reference analysis. + fn generate_acquires_map(&self, module: &ModuleEnv) -> BTreeMap> { + // Compute map with direct usage of resources + let mut usage_map = module + .get_functions() + .map(|f| (f.get_id(), self.get_direct_function_acquires(&f))) + .collect::>(); + // Now run a fixed-point loop: add resources used by called functions until there are no + // changes. + loop { + let mut changes = false; + for fun in module.get_functions() { + if let Some(callees) = fun.get_called_functions() { + let mut usage = usage_map[&fun.get_id()].clone(); + let count = usage.len(); + // Extend usage by that of callees from the same module. Acquires is only + // local to a module. + for callee in callees { + if callee.module_id == module.get_id() { + usage.extend(usage_map[&callee.id].iter().cloned()); + } + } + if usage.len() > count { + *usage_map.get_mut(&fun.get_id()).unwrap() = usage; + changes = true; + } + } + } + if !changes { + break; + } + } + usage_map + } + + fn get_direct_function_acquires(&self, fun: &FunctionEnv) -> BTreeSet { + let mut result = BTreeSet::new(); + let target = self.targets.get_target(fun, &FunctionVariant::Baseline); + for bc in target.get_bytecode() { + use Bytecode::*; + use Operation::*; + match bc { + Call(_, _, MoveFrom(mid, sid, ..), ..) + | Call(_, _, BorrowGlobal(mid, sid, ..), ..) + // GetGlobal from spec language, but cover it anyway. + | Call(_, _, GetGlobal(mid, sid, ..), ..) + if *mid == fun.module_env.get_id() => + { + result.insert(*sid); + }, + _ => {}, + } + } + result + } +} diff --git a/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.exp b/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.exp index 4ed3af5f47963..70d793c4bdbb5 100644 --- a/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.exp +++ b/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.exp @@ -3,16 +3,25 @@ module 0x42::fields { struct T { h: u64, } + struct G { + f: #0, + } struct S { f: u64, g: fields::T, } + private fun read_generic_val(x: fields::G): u64 { + select fields::G.f(x) + } private fun read_ref(x: &fields::S): u64 { select fields::T.h(select fields::S.g(x)) } private fun read_val(x: fields::S): u64 { select fields::T.h(select fields::S.g(x)) } + private fun write_generic_val(x: &mut fields::G,v: u64) { + select fields::G.f(x) = v + } private fun write_local_direct(): fields::S { { let x: fields::S = pack fields::S(0, pack fields::T(0)); @@ -38,12 +47,18 @@ module 0x42::fields { select fields::T.h(select fields::S.g(x)) = 42; x } + spec fun $read_generic_val(x: fields::G): u64 { + select fields::G.f(x) + } spec fun $read_ref(x: fields::S): u64 { select fields::T.h(select fields::S.g(x)) } spec fun $read_val(x: fields::S): u64 { select fields::T.h(select fields::S.g(x)) } + spec fun $write_generic_val(x: fields::G,v: u64) { + select fields::G.f(x) = v + } spec fun $write_local_direct(): fields::S; spec fun $write_local_via_ref(): fields::S; spec fun $write_param(x: &mut fields::S); @@ -52,6 +67,18 @@ module 0x42::fields { ============ initial bytecode ================ +[variant baseline] +fun fields::read_generic_val($t0: fields::G): u64 { + var $t1: u64 + var $t2: &fields::G + var $t3: &u64 + 0: $t2 := borrow_local($t0) + 1: $t3 := borrow_field>.f($t2) + 2: $t1 := read_ref($t3) + 3: return $t1 +} + + [variant baseline] fun fields::read_ref($t0: &fields::S): u64 { var $t1: u64 @@ -78,6 +105,15 @@ fun fields::read_val($t0: fields::S): u64 { } +[variant baseline] +fun fields::write_generic_val($t0: &mut fields::G, $t1: u64) { + var $t2: &mut u64 + 0: $t2 := borrow_field>.f($t0) + 1: write_ref($t2, $t1) + 2: return () +} + + [variant baseline] fun fields::write_local_direct(): fields::S { var $t0: fields::S diff --git a/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.move b/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.move index da6bea9d1d449..3d6d69aa4f304 100644 --- a/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.move +++ b/third_party/move/move-compiler-v2/tests/bytecode-generator/fields.move @@ -9,6 +9,10 @@ module 0x42::fields { h: u64 } + struct G { + f: X + } + fun read_val(x: S): u64 { x.g.h } @@ -38,4 +42,12 @@ module 0x42::fields { x.g.h = 42; x } + + fun read_generic_val(x: G): u64 { + x.f + } + + fun write_generic_val(x: &mut G, v: u64) { + x.f = v + } } diff --git a/third_party/move/move-compiler-v2/tests/checking/access_specifiers/access_err.exp b/third_party/move/move-compiler-v2/tests/checking/access_specifiers/access_err.exp index d509e3ec57029..2d07719a5be35 100644 --- a/third_party/move/move-compiler-v2/tests/checking/access_specifiers/access_err.exp +++ b/third_party/move/move-compiler-v2/tests/checking/access_specifiers/access_err.exp @@ -18,12 +18,6 @@ error: invalid access specifier: a wildcard cannot be followed by a non-wildcard 12 │ fun f3() acquires 0x42::*::S { │ ^^^^^^^^^^^ -error: type argument count mismatch (expected 1 but got 0) - ┌─ tests/checking/access_specifiers/access_err.move:15:23 - │ -15 │ fun f4() acquires G { - │ ^^ - error: undeclared `m::y` ┌─ tests/checking/access_specifiers/access_err.move:18:35 │ diff --git a/third_party/move/move-model/src/builder/exp_builder.rs b/third_party/move/move-model/src/builder/exp_builder.rs index 5934ad48e7577..d0e0087672787 100644 --- a/third_party/move/move-model/src/builder/exp_builder.rs +++ b/third_party/move/move-model/src/builder/exp_builder.rs @@ -906,8 +906,6 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo if is_wildcard(resource) { ResourceSpecifier::DeclaredInModule(module_id) } else { - // Construct an expansion type so we can feed it through the standard translation - // process. let mident = sp(specifier.loc, EA::ModuleIdent_ { address: *address, module: *module, @@ -916,16 +914,33 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo specifier.loc, EA::ModuleAccess_::ModuleAccess(mident, *resource), ); - let ety = sp( - specifier.loc, - EA::Type_::Apply(maccess, type_args.as_ref().cloned().unwrap_or_default()), - ); - let ty = self.translate_type(&ety); - if let Type::Struct(mid, sid, inst) = ty { - ResourceSpecifier::Resource(mid.qualified_inst(sid, inst)) + let sym = self.parent.module_access_to_qualified(&maccess); + if let Type::Struct(mid, sid, _) = self.parent.parent.lookup_type(&loc, &sym) { + if type_args.is_none() { + // If no type args are provided, we assume this is either a non-generic + // or a generic type without instantiation, which is a valid wild card. + ResourceSpecifier::Resource(mid.qualified_inst(sid, vec![])) + } else { + // Otherwise construct an expansion type so we can feed it through the standard translation + // process. + let ety = sp( + specifier.loc, + EA::Type_::Apply( + maccess, + type_args.as_ref().cloned().unwrap_or_default(), + ), + ); + let ty = self.translate_type(&ety); + if let Type::Struct(mid, sid, inst) = ty { + ResourceSpecifier::Resource(mid.qualified_inst(sid, inst)) + } else { + // errors reported + debug_assert!(self.parent.parent.env.has_errors()); + ResourceSpecifier::Any + } + } } else { - // errors reported - debug_assert!(self.parent.parent.env.has_errors()); + // error reported ResourceSpecifier::Any } } @@ -1788,13 +1803,13 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo Some(entries) => { if entries.len() != 1 { self.error( - loc, - &format!( - "Expect a unique spec function from lifted lambda: {}, found {}", - remapped_sym.display(self.symbol_pool()), - entries.len() - ), - ); + loc, + &format!( + "Expect a unique spec function from lifted lambda: {}, found {}", + remapped_sym.display(self.symbol_pool()), + entries.len() + ), + ); return Some(self.new_error_exp()); } entries.last().unwrap().clone() @@ -1835,12 +1850,12 @@ impl<'env, 'translator, 'module_translator> ExpTranslator<'env, 'translator, 'mo if full_arg_types.len() != spec_fun_entry.params.len() { self.error( - loc, - &format!( - "Parameter number mismatch on calling a spec function from lifted lambda: {},", - remapped_sym.display(self.symbol_pool()) - ), - ); + loc, + &format!( + "Parameter number mismatch on calling a spec function from lifted lambda: {},", + remapped_sym.display(self.symbol_pool()) + ), + ); return Some(self.new_error_exp()); } let param_type_error = full_arg_types