[move-2024][dev] Non-bytecode enums in compiler (MystenLabs#16060)

## Description This PR adds enums through the compiler down to bytecode generation, but does not generate bytecode for them (instead producing errors). This is to allow better testing and IDE integration as enums mature into alpha state. ## Test Plan A large number of enum tests. --- If your changes are not user-facing and do not break anything, you can skip the following section. Otherwise, please briefly describe what has changed under the Release Notes section. ### Type of Change (Check all that apply) - [ ] protocol change - [ ] user-visible impact - [ ] breaking change for a client SDKs - [ ] breaking change for FNs (FN binary must upgrade) - [ ] breaking change for validators or node operators (must upgrade binaries) - [ ] breaking change for on-chain data layout - [ ] necessitate either a data wipe or data migration ### Release notes --------- Co-authored-by: Timothy Zakian <[email protected]> Co-authored-by: Tim Zakian <[email protected]>
ArchieR7 · Apr 5, 2024 · 8ed9a52 · 8ed9a52
1 parent 72ea2a3
commit 8ed9a52
Show file tree

Hide file tree

Showing 307 changed files with 13,450 additions and 732 deletions.
diff --git a/external-crates/move/crates/move-analyzer/src/symbols.rs b/external-crates/move/crates/move-analyzer/src/symbols.rs
@@ -95,7 +95,7 @@ use move_compiler::{
     },
     linters::LintLevel,
     naming::ast::{StructDefinition, StructFields, TParam, Type, TypeName_, Type_, UseFuns},
-    parser::ast::{self as P, StructName},
+    parser::ast::{self as P, DatatypeName},
     shared::{unique_map::UniqueMap, Identifier, Name},
     typing::ast::{
         BuiltinFunction_, Exp, ExpListItem, Function, FunctionBody_, LValue, LValueList, LValue_,
@@ -1776,10 +1776,25 @@ impl<'a> ParsingSymbolicator<'a> {
                     self.type_symbols(&fun.signature.return_type);
                 }
                 MM::Struct(sdef) => match &sdef.fields {
-                    P::StructFields::Defined(v) => v.iter().for_each(|(_, t)| self.type_symbols(t)),
+                    P::StructFields::Named(v) => v.iter().for_each(|(_, t)| self.type_symbols(t)),
                     P::StructFields::Positional(v) => v.iter().for_each(|t| self.type_symbols(t)),
                     P::StructFields::Native(_) => (),
                 },
+                MM::Enum(edef) => {
+                    let P::EnumDefinition { variants, .. } = edef;
+                    for variant in variants {
+                        let P::VariantDefinition { fields, .. } = variant;
+                        match fields {
+                            P::VariantFields::Named(v) => {
+                                v.iter().for_each(|(_, t)| self.type_symbols(t))
+                            }
+                            P::VariantFields::Positional(v) => {
+                                v.iter().for_each(|t| self.type_symbols(t))
+                            }
+                            P::VariantFields::Empty => (),
+                        }
+                    }
+                }
                 MM::Use(use_decl) => self.use_decl_symbols(use_decl),
                 MM::Friend(fdecl) => self.chain_symbols(&fdecl.friend),
                 MM::Constant(c) => {
@@ -2104,10 +2119,20 @@ impl<'a> ParsingSymbolicator<'a> {
                 self.chain_symbols(chain);
                 match bindings {
                     P::FieldBindings::Named(v) => {
-                        v.iter().for_each(|(_, bind)| self.bind_symbols(bind))
+                        for symbol in v {
+                            match symbol {
+                                P::Ellipsis::Binder((_, x)) => self.bind_symbols(x),
+                                P::Ellipsis::Ellipsis(_) => (),
+                            }
+                        }
                     }
                     P::FieldBindings::Positional(v) => {
-                        v.iter().for_each(|bind| self.bind_symbols(bind))
+                        for symbol in v.iter() {
+                            match symbol {
+                                P::Ellipsis::Binder(x) => self.bind_symbols(x),
+                                P::Ellipsis::Ellipsis(_) => (),
+                            }
+                        }
                     }
                 }
             }
@@ -2436,6 +2461,9 @@ impl<'a> TypingSymbolicator<'a> {
                 self.unpack_symbols(define, ident, name, tparams, fields, scope);
             }
             LValue_::Ignore => (),
+            LValue_::UnpackVariant(..) | LValue_::BorrowUnpackVariant(..) => {
+                debug_assert!(false, "Enums are not supported by move analyzser.");
+            }
         }
     }
 
@@ -2444,7 +2472,7 @@ impl<'a> TypingSymbolicator<'a> {
         &mut self,
         define: bool,
         ident: &ModuleIdent,
-        name: &StructName,
+        name: &DatatypeName,
         tparams: &Vec<Type>,
         fields: &Fields<(Type, LValue)>,
         scope: &mut OrdMap<Symbol, LocalDef>,
@@ -2653,7 +2681,7 @@ impl<'a> TypingSymbolicator<'a> {
     fn pack_symbols(
         &mut self,
         ident: &ModuleIdent,
-        name: &StructName,
+        name: &DatatypeName,
         tparams: &Vec<Type>,
         fields: &Fields<(Type, Exp)>,
         scope: &mut OrdMap<Symbol, LocalDef>,

diff --git a/external-crates/move/crates/move-compiler/src/cfgir/ast.rs b/external-crates/move/crates/move-compiler/src/cfgir/ast.rs
@@ -6,10 +6,10 @@ use crate::{
     diagnostics::WarningFilters,
     expansion::ast::{Attributes, Friend, ModuleIdent, Mutability},
     hlir::ast::{
-        BaseType, Command, Command_, FunctionSignature, Label, SingleType, StructDefinition, Var,
-        Visibility,
+        BaseType, Command, Command_, EnumDefinition, FunctionSignature, Label, SingleType,
+        StructDefinition, Var, Visibility,
     },
-    parser::ast::{ConstantName, FunctionName, StructName, ENTRY_MODIFIER},
+    parser::ast::{ConstantName, DatatypeName, FunctionName, ENTRY_MODIFIER},
     shared::{ast_debug::*, unique_map::UniqueMap},
 };
 use move_core_types::runtime_value::MoveValue;
@@ -42,7 +42,8 @@ pub struct ModuleDefinition {
     /// `dependency_order` is the topological order/rank in the dependency graph.
     pub dependency_order: usize,
     pub friends: UniqueMap<ModuleIdent, Friend>,
-    pub structs: UniqueMap<StructName, StructDefinition>,
+    pub structs: UniqueMap<DatatypeName, StructDefinition>,
+    pub enums: UniqueMap<DatatypeName, EnumDefinition>,
     pub constants: UniqueMap<ConstantName, Constant>,
     pub functions: UniqueMap<FunctionName, Function>,
 }
@@ -172,6 +173,11 @@ fn remap_labels_cmd(remapping: &BTreeMap<Label, Label>, sp!(_, cmd_): &mut Comma
             *if_true = remapping[if_true];
             *if_false = remapping[if_false];
         }
+        VariantSwitch { arms, .. } => {
+            for (_, arm) in arms.iter_mut() {
+                *arm = remapping[arm];
+            }
+        }
     }
 }
 
@@ -201,6 +207,7 @@ impl AstDebug for ModuleDefinition {
             dependency_order,
             friends,
             structs,
+            enums,
             constants,
             functions,
         } = self;
@@ -223,6 +230,10 @@ impl AstDebug for ModuleDefinition {
             sdef.ast_debug(w);
             w.new_line();
         }
+        for edef in enums.key_cloned_iter() {
+            edef.ast_debug(w);
+            w.new_line();
+        }
         for cdef in constants.key_cloned_iter() {
             cdef.ast_debug(w);
             w.new_line();

diff --git a/external-crates/move/crates/move-compiler/src/cfgir/borrows/mod.rs b/external-crates/move/crates/move-compiler/src/cfgir/borrows/mod.rs
@@ -16,6 +16,7 @@ use crate::{
     shared::{unique_map::UniqueMap, CompilationEnv},
 };
 use move_proc_macros::growing_stack;
+
 use state::{Value, *};
 use std::{cell::RefCell, collections::BTreeMap, rc::Rc};
 
@@ -170,6 +171,12 @@ fn command(context: &mut Context, sp!(loc, cmd_): &Command) {
             let value = assert_single_value(exp(context, e));
             assert!(!value.is_ref());
         }
+        C::VariantSwitch { subject, .. } => {
+            let value = assert_single_value(exp(context, subject));
+            assert!(value.is_ref());
+            let diags = context.borrow_state.variant_switch(*loc, value);
+            context.add_diags(diags);
+        }
         C::IgnoreAndPop { exp: e, .. } => {
             let values = exp(context, e);
             context.borrow_state.release_values(values);
@@ -220,6 +227,36 @@ fn lvalue(context: &mut Context, sp!(loc, l_): &LValue, value: Value) {
                 .iter()
                 .for_each(|(_, l)| lvalue(context, l, Value::NonRef))
         }
+        L::UnpackVariant(_, _, unpack_type, _, _, fields) => match unpack_type {
+            UnpackType::ByValue => {
+                assert!(!value.is_ref());
+                fields
+                    .iter()
+                    .for_each(|(_, l)| lvalue(context, l, Value::NonRef))
+            }
+            UnpackType::ByImmRef => {
+                assert!(value.is_ref());
+                let (diags, fvs) = context
+                    .borrow_state
+                    .borrow_variant_fields(*loc, false, value, fields);
+                context.add_diags(diags);
+                assert!(fvs.len() == fields.len());
+                fvs.into_iter()
+                    .zip(fields.iter())
+                    .for_each(|(fv, (_, l))| lvalue(context, l, fv));
+            }
+            UnpackType::ByMutRef => {
+                assert!(value.is_ref());
+                let (diags, fvs) = context
+                    .borrow_state
+                    .borrow_variant_fields(*loc, true, value, fields);
+                context.add_diags(diags);
+                assert!(fvs.len() == fields.len());
+                fvs.into_iter()
+                    .zip(fields.iter())
+                    .for_each(|(fv, (_, l))| lvalue(context, l, fv));
+            }
+        },
     }
 }
 
@@ -323,6 +360,13 @@ fn exp(context: &mut Context, parent_e: &Exp) -> Values {
             });
             svalue()
         }
+        E::PackVariant(_, _, _, fields) => {
+            fields.iter().for_each(|(_, _, e)| {
+                let arg = exp(context, e);
+                assert!(!assert_single_value(arg).is_ref());
+            });
+            svalue()
+        }
 
         E::Multiple(es) => es.iter().flat_map(|e| exp(context, e)).collect(),
 

diff --git a/external-crates/move/crates/move-compiler/src/cfgir/borrows/state.rs b/external-crates/move/crates/move-compiler/src/cfgir/borrows/state.rs
@@ -406,7 +406,12 @@ impl BorrowState {
             code,
             move || {
                 let local_str = match display_var(local.value()) {
-                    DisplayVar::Tmp => panic!("ICE invalid use of tmp local {}", local.value()),
+                    DisplayVar::Tmp => panic!(
+                        "ICE invalid use of tmp local {} with borrows {:#?}",
+                        local.value(),
+                        borrows
+                    ),
+                    DisplayVar::MatchTmp(s) => s,
                     DisplayVar::Orig(s) => s,
                 };
                 format!("Invalid {} of variable '{}'", verb, local_str)
@@ -510,6 +515,7 @@ impl BorrowState {
                     || {
                         let case = match display_var(local.value()) {
                             DisplayVar::Orig(v) => format!("Local variable '{v}'"),
+                            DisplayVar::MatchTmp(_) => "Local value".to_string(),
                             DisplayVar::Tmp => "Local value".to_string(),
                         };
                         format!("Invalid return. {case} is still being borrowed.")
@@ -580,6 +586,9 @@ impl BorrowState {
                             DisplayVar::Tmp => {
                                 panic!("ICE invalid use tmp local {}", local.value())
                             }
+                            DisplayVar::MatchTmp(s) => {
+                                panic!("ICE invalid use match tmp {}: {}", s, local.value())
+                            }
                             DisplayVar::Orig(s) => s,
                         };
                         format!("Ambiguous usage of variable '{}'", vstr)
@@ -590,6 +599,9 @@ impl BorrowState {
                         DisplayVar::Tmp => {
                             panic!("ICE invalid use tmp local {}", local.value())
                         }
+                        DisplayVar::MatchTmp(s) => {
+                            panic!("ICE invalid use match tmp {}: {}", s, local.value())
+                        }
                         DisplayVar::Orig(s) => s,
                     };
                     let tip = format!(
@@ -653,7 +665,8 @@ impl BorrowState {
     pub fn borrow_local(&mut self, loc: Loc, mut_: bool, local: &Var) -> (Diagnostics, Value) {
         assert!(
             !self.locals.get(local).unwrap().is_ref(),
-            "ICE borrow ref {:#?}. Should have been caught in typing",
+            "ICE borrow ref of {:?} at {:#?}. Should have been caught in typing",
+            local,
             loc
         );
         let new_id = self.declare_new_ref(loc, mut_);
@@ -783,6 +796,77 @@ impl BorrowState {
         (diags, Value::Ref(field_borrow_id))
     }
 
+    pub fn borrow_variant_fields(
+        &mut self,
+        loc: Loc,
+        mut_: bool,
+        rvalue: Value,
+        fields: &[(Field, LValue)],
+    ) -> (Diagnostics, Vec<Value>) {
+        let mut diags = Diagnostics::new();
+        let id = match rvalue {
+            Value::NonRef => {
+                assert!(
+                    self.prev_had_errors,
+                    "ICE borrow checking failed {:#?}",
+                    loc
+                );
+                return (
+                    Diagnostics::new(),
+                    fields.iter().map(|_| Value::NonRef).collect::<Vec<_>>(),
+                );
+            }
+            Value::Ref(id) => id,
+        };
+        let copy_parent = Some(id);
+        let fvs = fields
+            .iter()
+            .map(|(field, _)| {
+                let new_diags = if mut_ {
+                    let msg = || format!("Invalid mutable borrow at field '{}'.", field);
+                    let (full_borrows, _field_borrows) = self.borrows.borrowed_by(id);
+                    // Any field borrows will be factored out
+                    Self::borrow_error(
+                        &self.borrows,
+                        loc,
+                        &full_borrows,
+                        &BTreeMap::new(),
+                        ReferenceSafety::MutOwns,
+                        msg,
+                    )
+                    .into()
+                } else {
+                    let msg = || format!("Invalid immutable borrow at field '{}'.", field);
+                    self.readable(loc, ReferenceSafety::RefTrans, msg, id, Some(field))
+                };
+                diags.extend(new_diags);
+                let field_borrow_id = self.declare_new_ref_impl(loc, mut_, copy_parent);
+                self.add_field_borrow(loc, id, *field, field_borrow_id);
+                Value::Ref(field_borrow_id)
+            })
+            .collect::<Vec<_>>();
+        self.release(id);
+        (diags, fvs)
+    }
+
+    pub fn variant_switch(&mut self, loc: Loc, subject: Value) -> Diagnostics {
+        let id = match subject {
+            Value::NonRef => {
+                assert!(
+                    self.prev_had_errors,
+                    "ICE borrow checking failed {:#?}",
+                    loc
+                );
+                return Diagnostics::new();
+            }
+            Value::Ref(id) => id,
+        };
+        let msg = || "Invalid immutable borrow of match subject".to_string();
+        let diags = self.readable(loc, ReferenceSafety::RefTrans, msg, id, None);
+        self.release(id);
+        diags
+    }
+
     pub fn call(&mut self, loc: Loc, args: Values, return_ty: &Type) -> (Diagnostics, Values) {
         let mut diags = Diagnostics::new();
 
@@ -854,8 +938,22 @@ impl BorrowState {
             }
         }
         all_refs.remove(&Self::LOCAL_ROOT);
-        assert!(all_refs.is_empty());
-
+        if !all_refs.is_empty() {
+            for ref_ in all_refs {
+                println!("had ref: {:?}", ref_);
+            }
+            println!("borrow graph:");
+            self.borrows.display();
+            println!("locals:");
+            for (_, local_, value) in &self.locals {
+                println!("{} -> {:?}", local_, value);
+            }
+            println!("id map:");
+            for (key, value) in &id_map {
+                println!("{:?} -> {:?}", key, value);
+            }
+            panic!("Had some refs left over");
+        }
         self.locals
             .iter_mut()
             .for_each(|(_, _, v)| v.remap_refs(&id_map));

diff --git a/external-crates/move/crates/move-compiler/src/cfgir/cfg.rs b/external-crates/move/crates/move-compiler/src/cfgir/cfg.rs
@@ -352,12 +352,18 @@ fn maybe_unmark_infinite_loop_starts(
         } if cur_loop_end.equals(*if_true) || cur_loop_end.equals(*if_false) => {
             infinite_loop_starts.remove(&cur_loop_start);
         }
+        C::VariantSwitch { arms, .. }
+            if arms.iter().any(|(_, target)| cur_loop_end.equals(*target)) =>
+        {
+            infinite_loop_starts.remove(&cur_loop_start);
+        }
         C::Return { .. } | C::Abort(_) => {
             infinite_loop_starts.remove(&cur_loop_start);
         }
 
         C::Jump { .. }
         | C::JumpIf { .. }
+        | C::VariantSwitch { .. }
         | C::Assign(_, _, _)
         | C::Mutate(_, _)
         | C::IgnoreAndPop { .. } => (),