Remove () expressions and () type except returns.

() serves no purpose beyond indicating the absence of return values, at
least until/unless we have some kind of type polymorphism.

Author: mikebenfield

compiler/ast/src/statement/return_.rs

@@ -33,7 +33,11 @@ pub struct ReturnStatement {
 
 impl fmt::Display for ReturnStatement {
     fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
-        write!(f, "return {}", self.expression)
+        if let Expression::Unit(..) = self.expression {
+            write!(f, "return")
+        } else {
+            write!(f, "return {}", self.expression)
+        }
     }
 }
 

compiler/parser/src/parser/expression.rs

@@ -608,18 +608,20 @@ impl<N: Network> ParserContext<'_, N> {
 
         let (mut elements, trailing, span) = self.parse_expr_tuple()?;
 
-        match elements.len() {
-            // If the tuple expression is empty, return a `UnitExpression`.
-            0 => Ok(Expression::Unit(UnitExpression { span, id: self.node_builder.next_id() })),
-            1 => match trailing {
+        match (elements.len(), trailing) {
+            (0, _) | (1, true) => {
+                // A tuple with 0 or 1 elements - emit an error since tuples must have at least two elements.
+                Err(ParserError::tuple_must_have_at_least_two_elements("expression", span).into())
+            }
+            (1, false) => {
                 // If there is one element in the tuple but no trailing comma, e.g `(foo)`, return the element.
-                false => Ok(elements.swap_remove(0)),
-                // If there is one element in the tuple and a trailing comma, e.g `(foo,)`, emit an error since tuples must have at least two elements.
-                true => Err(ParserError::tuple_must_have_at_least_two_elements("expression", span).into()),
-            },
-            // Otherwise, return a tuple expression.
-            // Note: This is the only place where `TupleExpression` is constructed in the parser.
-            _ => Ok(Expression::Tuple(TupleExpression { elements, span, id: self.node_builder.next_id() })),
+                Ok(elements.remove(0))
+            }
+            _ => {
+                // Otherwise, return a tuple expression.
+                // Note: This is the only place where `TupleExpression` is constructed in the parser.
+                Ok(Expression::Tuple(TupleExpression { elements, span, id: self.node_builder.next_id() }))
+            }
         }
     }
 

compiler/passes/src/code_generation/visit_statements.rs

@@ -80,59 +80,56 @@ impl<'a> CodeGenerator<'a> {
     }
 
     fn visit_return(&mut self, input: &'a ReturnStatement) -> String {
-        let outputs = match input.expression {
+        if let Expression::Unit(..) = input.expression {
             // Skip empty return statements.
-            Expression::Unit(_) => String::new(),
-            _ => {
-                let (operand, mut expression_instructions) = self.visit_expression(&input.expression);
-                // Get the output type of the function.
-                let output = self.current_function.unwrap().output.iter();
-                // If the operand string is empty, initialize an empty vector.
-                let operand_strings = match operand.is_empty() {
-                    true => vec![],
-                    false => operand.split(' ').collect_vec(),
-                };
+            return String::new();
+        }
 
-                let mut future_output = String::new();
-                let mut instructions = operand_strings
-                    .iter()
-                    .zip_eq(output)
-                    .map(|(operand, output)| {
-                        // Transitions outputs with no mode are private.
-                        // Note that this unwrap is safe because we set the variant before traversing the function.
-                        let visibility = match (self.variant.unwrap().is_transition(), output.mode) {
-                            (true, Mode::None) => Mode::Private,
-                            (_, mode) => mode,
-                        };
-
-                        if let Type::Future(_) = output.type_ {
-                            future_output = format!(
-                                "    output {} as {}.aleo/{}.future;\n",
-                                operand,
-                                self.program_id.unwrap().name,
-                                self.current_function.unwrap().identifier,
-                            );
-                            String::new()
-                        } else {
-                            format!(
-                                "    output {} as {};\n",
-                                operand,
-                                self.visit_type_with_visibility(&output.type_, visibility)
-                            )
-                        }
-                    })
-                    .join("");
+        let (operand, mut expression_instructions) = self.visit_expression(&input.expression);
+        // Get the output type of the function.
+        let output = self.current_function.unwrap().output.iter();
+        // If the operand string is empty, initialize an empty vector.
+        let operand_strings = match operand.is_empty() {
+            true => vec![],
+            false => operand.split(' ').collect_vec(),
+        };
+
+        let mut future_output = String::new();
+        let mut instructions = operand_strings
+            .iter()
+            .zip_eq(output)
+            .map(|(operand, output)| {
+                // Transitions outputs with no mode are private.
+                // Note that this unwrap is safe because we set the variant before traversing the function.
+                let visibility = match (self.variant.unwrap().is_transition(), output.mode) {
+                    (true, Mode::None) => Mode::Private,
+                    (_, mode) => mode,
+                };
 
-                // Insert future output at the end.
-                instructions.push_str(&future_output);
+                if let Type::Future(_) = output.type_ {
+                    future_output = format!(
+                        "    output {} as {}.aleo/{}.future;\n",
+                        operand,
+                        self.program_id.unwrap().name,
+                        self.current_function.unwrap().identifier,
+                    );
+                    String::new()
+                } else {
+                    format!(
+                        "    output {} as {};\n",
+                        operand,
+                        self.visit_type_with_visibility(&output.type_, visibility)
+                    )
+                }
+            })
+            .join("");
 
-                expression_instructions.push_str(&instructions);
+        // Insert future output at the end.
+        instructions.push_str(&future_output);
 
-                expression_instructions
-            }
-        };
+        expression_instructions.push_str(&instructions);
 
-        outputs
+        expression_instructions
     }
 
     fn visit_definition(&mut self, _input: &'a DefinitionStatement) -> String {

compiler/passes/src/type_checking/check_expressions.rs

@@ -996,12 +996,7 @@ impl<'a, N: Network> ExpressionVisitor<'a> for TypeChecker<'a, N> {
         ty
     }
 
-    fn visit_unit(&mut self, input: &'a UnitExpression, additional: &Self::AdditionalInput) -> Self::Output {
-        // Unit expression are only allowed inside a return statement.
-        if !self.scope_state.is_return {
-            self.emit_err(TypeCheckerError::unit_expression_only_in_return_statements(input.span()));
-        }
-        self.maybe_assert_type(&Type::Unit, additional, input.span());
-        Type::Unit
+    fn visit_unit(&mut self, _input: &'a UnitExpression, _additional: &Self::AdditionalInput) -> Self::Output {
+        unreachable!("Unit expressions should not exist in normal code");
     }
 }

compiler/passes/src/type_checking/check_statements.rs

@@ -222,8 +222,6 @@ impl<'a, N: Network> StatementVisitor<'a> for TypeChecker<'a, N> {
 
         // Check that the type of the definition is not a unit type, singleton tuple type, or nested tuple type.
         match &input.type_ {
-            // If the type is an empty tuple, return an error.
-            Type::Unit => self.emit_err(TypeCheckerError::lhs_must_be_identifier_or_tuple(input.span)),
             // If the type is a singleton tuple, return an error.
             Type::Tuple(tuple) => match tuple.length() {
                 0 | 1 => unreachable!("Parsing guarantees that tuple types have at least two elements."),
@@ -421,20 +419,12 @@ impl<'a, N: Network> StatementVisitor<'a> for TypeChecker<'a, N> {
         // Set the `has_return` flag.
         self.scope_state.has_return = true;
 
-        // Check that the return expression is not a nested tuple.
-        if let Expression::Tuple(TupleExpression { elements, .. }) = &input.expression {
-            for element in elements {
-                if matches!(element, Expression::Tuple(_)) {
-                    self.emit_err(TypeCheckerError::nested_tuple_expression(element.span()));
-                }
-            }
-        }
-
-        // Set the `is_return` flag. This is necessary to allow unit expressions in the return statement.
-        self.scope_state.is_return = true;
         // Type check the associated expression.
-        self.visit_expression(&input.expression, &return_type);
-        // Unset the `is_return` flag.
-        self.scope_state.is_return = false;
+        if let Expression::Unit(..) = input.expression {
+            // TODO - do will this error message be appropriate?
+            self.maybe_assert_type(&Type::Unit, &return_type, input.expression.span());
+        } else {
+            self.visit_expression(&input.expression, &return_type);
+        }
     }
 }

compiler/passes/src/type_checking/checker.rs

@@ -856,29 +856,30 @@ impl<'a, N: Network> TypeChecker<'a, N> {
     }
 
     /// Emits an error if the type or its constituent types is not valid.
-    pub(crate) fn assert_type_is_valid(&mut self, type_: &Type, span: Span) -> bool {
-        let mut is_valid = true;
+    pub(crate) fn assert_type_is_valid(&mut self, type_: &Type, span: Span) {
         match type_ {
+            // Unit types may only appear as the return type of a function.
+            Type::Unit => {
+                self.emit_err(TypeCheckerError::unit_type_only_return(span));
+            }
             // String types are temporarily disabled.
             Type::String => {
-                is_valid = false;
                 self.emit_err(TypeCheckerError::strings_are_not_supported(span));
             }
             // Check that the named composite type has been defined.
             Type::Composite(struct_) if self.lookup_struct(struct_.program, struct_.id.name).is_none() => {
-                is_valid = false;
                 self.emit_err(TypeCheckerError::undefined_type(struct_.id.name, span));
             }
             // Check that the constituent types of the tuple are valid.
             Type::Tuple(tuple_type) => {
                 for type_ in tuple_type.elements().iter() {
-                    is_valid &= self.assert_type_is_valid(type_, span)
+                    self.assert_type_is_valid(type_, span);
                 }
             }
             // Check that the constituent types of mapping are valid.
             Type::Mapping(mapping_type) => {
-                is_valid &= self.assert_type_is_valid(&mapping_type.key, span);
-                is_valid &= self.assert_type_is_valid(&mapping_type.value, span);
+                self.assert_type_is_valid(&mapping_type.key, span);
+                self.assert_type_is_valid(&mapping_type.value, span);
             }
             // Check that the array element types are valid.
             Type::Array(array_type) => {
@@ -909,11 +910,10 @@ impl<'a, N: Network> TypeChecker<'a, N> {
                     }
                     _ => {} // Do nothing.
                 }
-                is_valid &= self.assert_type_is_valid(array_type.element_type(), span)
+                self.assert_type_is_valid(array_type.element_type(), span);
             }
             _ => {} // Do nothing.
         }
-        is_valid
     }
 
     /// Emits an error if the type is not a mapping.
@@ -1067,17 +1067,19 @@ impl<'a, N: Network> TypeChecker<'a, N> {
                     }
                 }
             }
-            // Check that the type of output is defined.
-            if self.assert_type_is_valid(&function_output.type_, function_output.span) {
-                // If the function is not a transition function, then it cannot output a record.
-                if let Type::Composite(struct_) = function_output.type_.clone() {
-                    if !function.variant.is_transition()
-                        && self.lookup_struct(struct_.program, struct_.id.name).unwrap().is_record
-                    {
-                        self.emit_err(TypeCheckerError::function_cannot_input_or_output_a_record(function_output.span));
-                    }
+
+            // Check that the output type is valid.
+            self.assert_type_is_valid(&function_output.type_, function_output.span);
+
+            // If the function is not a transition function, then it cannot output a record.
+            if let Type::Composite(struct_) = function_output.type_.clone() {
+                if !function.variant.is_transition()
+                    && self.lookup_struct(struct_.program, struct_.id.name).map_or(false, |s| s.is_record)
+                {
+                    self.emit_err(TypeCheckerError::function_cannot_input_or_output_a_record(function_output.span));
                 }
             }
+
             // Check that the type of the output is not a tuple. This is necessary to forbid nested tuples.
             if matches!(&function_output.type_, Type::Tuple(_)) {
                 self.emit_err(TypeCheckerError::nested_tuple_type(function_output.span))

compiler/passes/src/type_checking/scope_state.rs

@@ -25,8 +25,6 @@ pub struct ScopeState {
     pub(crate) variant: Option<Variant>,
     /// Whether or not the function that we are currently traversing has a return statement.
     pub(crate) has_return: bool,
-    /// Whether or not we are currently traversing a return statement.
-    pub(crate) is_return: bool,
     /// Current program name.
     pub(crate) program_name: Option<Symbol>,
     /// Whether or not we are currently traversing a stub.
@@ -50,7 +48,6 @@ impl ScopeState {
             function: None,
             variant: None,
             has_return: false,
-            is_return: false,
             program_name: None,
             is_stub: true,
             futures: IndexMap::new(),

errors/src/errors/type_checker/type_checker_error.rs

@@ -418,8 +418,7 @@ create_messages!(
         help: None,
     }
 
-    // TODO: Consider changing this to a warning.
-
+    // TODO This error is unused. Remove it in a future version.
     @formatted
     assign_unit_expression_to_variable {
         args: (),
@@ -476,6 +475,7 @@ create_messages!(
         help: None,
     }
 
+    // TODO This error is unused. Remove it in a future version.
     @formatted
     unit_expression_only_in_return_statements {
         args: (),
@@ -976,4 +976,11 @@ create_messages!(
         ),
         help: Some("Valid second operands are `u8`, `u16`, or `u32`".into()),
     }
+
+    @formatted
+    unit_type_only_return {
+        args: (),
+        msg: "The unit type () may appear only as the return type of a function.".to_string(),
+        help: None,
+    }
 );

tests/expectations/compiler/array/array_with_units_fail.out

@@ -1,30 +1,15 @@
 namespace = "Compile"
 expectation = "Fail"
 outputs = ["""
-Error [ETYC0372055]: The left-hand side of a `DefinitionStatement` can only be an identifier or tuple. Note that a tuple must contain at least two elements.
-    --> compiler-test:7:9
-     |
-   7 |         const A: () = ();
-     |         ^^^^^^^^^^^^^^^^
 Error [ETYC0372070]: The value of a const declaration must be a literal
     --> compiler-test:7:9
      |
-   7 |         const A: () = ();
-     |         ^^^^^^^^^^^^^^^^
-Error [ETYC0372056]: Unit expressions can only be used in return statements.
-    --> compiler-test:7:23
-     |
-   7 |         const A: () = ();
-     |                       ^^
-Error [ETYC0372070]: The value of a const declaration must be a literal
-    --> compiler-test:8:9
-     |
-   8 |         const B: u8 = ((1u8,1u8),1u8);
+   7 |         const B: u8 = ((1u8,1u8),1u8);
      |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 Error [ETYC0372023]: Tuples must be explicitly typed in Leo
-    --> compiler-test:8:23
+    --> compiler-test:7:23
      |
-   8 |         const B: u8 = ((1u8,1u8),1u8);
+   7 |         const B: u8 = ((1u8,1u8),1u8);
      |                       ^^^^^^^^^^^^^^^
      |
      = The function definition must match the function return statement

tests/expectations/compiler/constants/const_type_error_fail.out

@@ -1,7 +1,7 @@
 namespace = "Compile"
 expectation = "Fail"
 outputs = ["""
-Error [ETYC0372055]: The left-hand side of a `DefinitionStatement` can only be an identifier or tuple. Note that a tuple must contain at least two elements.
+Error [ETYC0372123]: The unit type () may appear only as the return type of a function.
     --> compiler-test:11:9
      |
   11 |         let result: () = Mapping::set(amounts, addr, amount);