Merge pull request #459 from sbillig/lowering-context

Move tuple and list expr collection to lowering pass; support lowering of nested tuples

.github/workflows/main.yml

@@ -7,7 +7,6 @@ on:
       - v*
 
   pull_request:
-    branches: [master]
 
 env:
   CARGO_TERM_COLOR: always

analyzer/src/context.rs

@@ -2,7 +2,7 @@ use crate::builtins::GlobalMethod;
 use crate::errors::CannotMove;
 use crate::namespace::events::EventDef;
 use crate::namespace::scopes::{ContractFunctionDef, ContractScope, ModuleScope, Shared};
-use crate::namespace::types::{Array, Contract, FixedSize, Struct, Tuple, Type};
+use crate::namespace::types::{Contract, FixedSize, Struct, Type};
 pub use fe_common::diagnostics::Label;
 use fe_common::diagnostics::{Diagnostic, Severity};
 use fe_common::files::SourceFileId;
@@ -52,8 +52,6 @@ pub struct ContractAttributes {
     pub init_function: Option<FunctionAttributes>,
     /// Events that have been defined by the user.
     pub events: Vec<EventDef>,
-    /// List expressions that the contract uses
-    pub list_expressions: BTreeSet<Array>,
     /// Static strings that the contract defines
     pub string_literals: BTreeSet<String>,
     /// Structs that have been defined by the user
@@ -121,7 +119,6 @@ impl From<Shared<ContractScope>> for ContractAttributes {
                 .values()
                 .map(|event| event.to_owned())
                 .collect::<Vec<EventDef>>(),
-            list_expressions: scope.borrow().list_expressions.clone(),
             string_literals: scope.borrow().string_defs.clone(),
             structs,
             external_contracts,
@@ -224,17 +221,12 @@ impl From<ContractFunctionDef> for FunctionAttributes {
 pub struct ModuleAttributes {
     /// Type definitions in a module.
     pub type_defs: BTreeMap<String, Type>,
-    /// Tuples that were used inside of a module.
-    ///
-    /// BTreeSet is used for ordering, this way items are retrieved in the same order every time.
-    pub tuples_used: BTreeSet<Tuple>,
 }
 
 impl From<Shared<ModuleScope>> for ModuleAttributes {
     fn from(scope: Shared<ModuleScope>) -> Self {
         Self {
             type_defs: scope.borrow().type_defs.clone(),
-            tuples_used: scope.borrow().tuples_used.clone(),
         }
     }
 }

analyzer/src/namespace/scopes.rs

@@ -1,6 +1,6 @@
 use crate::errors::AlreadyDefined;
 use crate::namespace::events::EventDef;
-use crate::namespace::types::{Array, FixedSize, Tuple, Type};
+use crate::namespace::types::{FixedSize, Type};
 use std::cell::RefCell;
 use std::collections::btree_map::Entry;
 use std::collections::{BTreeMap, BTreeSet};
@@ -27,10 +27,6 @@ pub struct ContractFieldDef {
 pub struct ModuleScope {
     /// Type definitions in a module.
     pub type_defs: BTreeMap<String, Type>,
-    /// Tuples that were used inside of a module.
-    ///
-    /// BTreeSet is used for ordering, this way items are retrieved in the same order every time.
-    pub tuples_used: BTreeSet<Tuple>,
 }
 
 #[derive(Clone, Debug, PartialEq)]
@@ -41,7 +37,6 @@ pub struct ContractScope {
     pub event_defs: BTreeMap<String, EventDef>,
     pub field_defs: BTreeMap<String, ContractFieldDef>,
     pub function_defs: BTreeMap<String, ContractFunctionDef>,
-    pub list_expressions: BTreeSet<Array>,
     pub string_defs: BTreeSet<String>,
     pub created_contracts: BTreeSet<String>,
     num_fields: usize,
@@ -89,7 +84,6 @@ impl ModuleScope {
     pub fn new() -> Shared<Self> {
         Rc::new(RefCell::new(ModuleScope {
             type_defs: BTreeMap::new(),
-            tuples_used: BTreeSet::new(),
         }))
     }
 
@@ -123,7 +117,6 @@ impl ContractScope {
             string_defs: BTreeSet::new(),
             interface: vec![],
             created_contracts: BTreeSet::new(),
-            list_expressions: BTreeSet::new(),
             num_fields: 0,
         }))
     }
@@ -210,11 +203,6 @@ impl ContractScope {
     pub fn add_created_contract(&mut self, name: &str) {
         self.created_contracts.insert(name.to_owned());
     }
-
-    /// Add the array type of a list expression that was used within the contract.
-    pub fn add_used_list_expression(&mut self, typ: Array) {
-        self.list_expressions.insert(typ);
-    }
 }
 
 impl BlockScope {

analyzer/src/namespace/types.rs

@@ -290,39 +290,57 @@ pub trait TypeDowncast {
     fn as_int(&self) -> Option<Integer>;
 }
 
-impl TypeDowncast for Option<&Type> {
+impl TypeDowncast for Type {
     fn as_array(&self) -> Option<&Array> {
         match self {
-            Some(Type::Array(inner)) => Some(inner),
+            Type::Array(inner) => Some(inner),
             _ => None,
         }
     }
     fn as_tuple(&self) -> Option<&Tuple> {
         match self {
-            Some(Type::Tuple(inner)) => Some(inner),
+            Type::Tuple(inner) => Some(inner),
             _ => None,
         }
     }
     fn as_string(&self) -> Option<&FeString> {
         match self {
-            Some(Type::String(inner)) => Some(inner),
+            Type::String(inner) => Some(inner),
             _ => None,
         }
     }
     fn as_map(&self) -> Option<&Map> {
         match self {
-            Some(Type::Map(inner)) => Some(inner),
+            Type::Map(inner) => Some(inner),
             _ => None,
         }
     }
     fn as_int(&self) -> Option<Integer> {
         match self {
-            Some(Type::Base(Base::Numeric(int))) => Some(*int),
+            Type::Base(Base::Numeric(int)) => Some(*int),
             _ => None,
         }
     }
 }
 
+impl TypeDowncast for Option<&Type> {
+    fn as_array(&self) -> Option<&Array> {
+        self.and_then(|t| t.as_array())
+    }
+    fn as_tuple(&self) -> Option<&Tuple> {
+        self.and_then(|t| t.as_tuple())
+    }
+    fn as_string(&self) -> Option<&FeString> {
+        self.and_then(|t| t.as_string())
+    }
+    fn as_map(&self) -> Option<&Map> {
+        self.and_then(|t| t.as_map())
+    }
+    fn as_int(&self) -> Option<Integer> {
+        self.and_then(|t| t.as_int())
+    }
+}
+
 impl From<FixedSize> for Type {
     fn from(value: FixedSize) -> Self {
         match value {
@@ -774,7 +792,12 @@ impl SafeNames for Tuple {
             .collect::<Vec<String>>();
         let joined_names = field_names.join("_");
 
-        format!("tuple_{}", joined_names)
+        // The trailing `_` denotes the end of the tuple, to differentiate between
+        // different tuple nestings. Eg
+        // (A, (B, C), D) => tuple_A_tuple_B_C__D_
+        // (A, (B, C, D)) => tuple_A_tuple_B_C_D__
+        // Conceptually, each paren and comma is replaced with an underscore.
+        format!("tuple_{}_", joined_names)
     }
 }
 

analyzer/src/traversal/expressions.rs

@@ -126,13 +126,6 @@ pub fn expr_list(
         size: elts.len(),
         inner: inner_type,
     };
-
-    scope
-        .borrow()
-        .contract_scope()
-        .borrow_mut()
-        .add_used_list_expression(array_typ.clone());
-
     Ok(ExpressionAttributes {
         typ: Type::Array(array_typ),
         location: Location::Memory,
@@ -245,14 +238,6 @@ fn expr_tuple(
         let tuple = Tuple {
             items: Vec1::try_from_vec(tuple_types).expect("tuple is empty"),
         };
-
-        scope
-            .borrow()
-            .module_scope()
-            .borrow_mut()
-            .tuples_used
-            .insert(tuple.clone());
-
         Ok(ExpressionAttributes::new(
             Type::Tuple(tuple),
             Location::Memory,

analyzer/src/traversal/types.rs

@@ -133,13 +133,6 @@ pub fn type_desc(
     };
 
     context.add_type_desc(desc, typ.clone());
-    if let Type::Tuple(tuple) = &typ {
-        scope
-            .module_scope()
-            .borrow_mut()
-            .tuples_used
-            .insert(tuple.to_owned());
-    }
     Ok(typ)
 }
 

compiler/Cargo.toml

@@ -28,6 +28,7 @@ maplit = "1.0.2"
 once_cell = "1.5.2"
 vec1 = "1.8.0"
 num-bigint = "0.3.1"
+indexmap = "1.6.2"
 
 [dev-dependencies]
 evm-runtime = "0.18"

compiler/src/lib.rs

@@ -35,27 +35,27 @@ pub fn compile(
     }
 
     // analyze source code
-    let mut context = match fe_analyzer::analyze(&fe_module, file_id) {
+    let analysis = match fe_analyzer::analyze(&fe_module, file_id) {
         Ok(_) if !errors.is_empty() => return Err(CompileError { errors }),
-        Ok(context) => context,
+        Ok(analysis) => analysis,
         Err(err) => {
             errors.push(ErrorKind::Analyzer(err));
             return Err(CompileError { errors });
         }
     };
 
     // build abi
-    let json_abis = abi::build(&context, &fe_module)?;
+    let json_abis = abi::build(&analysis, &fe_module)?;
 
     // lower the AST
-    let lowered_fe_module = lowering::lower(&mut context, fe_module.clone());
+    let lowered_fe_module = lowering::lower(&analysis, fe_module.clone());
 
     // analyze the lowered AST
-    let context =
+    let analysis =
         fe_analyzer::analyze(&lowered_fe_module, file_id).expect("failed to analyze lowered AST");
 
     // compile to yul
-    let yul_contracts = yul::compile(&context, &lowered_fe_module);
+    let yul_contracts = yul::compile(&analysis, &lowered_fe_module);
 
     // compile to bytecode if required
     #[cfg(feature = "solc-backend")]

compiler/src/lowering/context.rs

@@ -0,0 +1,48 @@
+use fe_analyzer::context::Context as AnalyzerContext;
+use fe_analyzer::namespace::types::{Array, Tuple};
+use indexmap::IndexSet;
+use std::collections::BTreeSet;
+
+pub struct ModuleContext<'a> {
+    pub analysis: &'a AnalyzerContext,
+
+    /// Tuples that were used inside of a module,
+    /// and the generated name of the resulting struct.
+    pub tuples: IndexSet<Tuple>,
+
+    /// Holds fresh id for [`ModuleContext::make_unique_name`]
+    fresh_id: u64,
+}
+
+impl<'a> ModuleContext<'a> {
+    pub fn new(analysis: &'a AnalyzerContext) -> Self {
+        Self {
+            analysis,
+            tuples: IndexSet::new(),
+            fresh_id: 0,
+        }
+    }
+
+    /// Makes a unique name from the given name, keeping it as readable as possible.
+    pub fn make_unique_name(&mut self, name: &str) -> String {
+        let id = self.fresh_id;
+        self.fresh_id += 1;
+        format!("${}_{}", name, id)
+    }
+}
+
+// This is contract context, but it's used all over so it has a short name.
+pub struct Context<'a, 'b> {
+    pub module: &'a mut ModuleContext<'b>,
+    /// List expressions that the contract uses
+    pub list_expressions: BTreeSet<Array>,
+}
+
+impl<'a, 'b> Context<'a, 'b> {
+    pub fn new(module: &'a mut ModuleContext<'b>) -> Self {
+        Self {
+            module,
+            list_expressions: BTreeSet::new(),
+        }
+    }
+}