Correctly handle diverging expressions in blocks (FuelLabs#2627)

* Add pretty-printing debugging to sway-ir * trivial refactor/cleanup in sway-ir * add Value::is_diverging method * improve divergence-handling in type-checking and ir generation * make returning/reverting asm blocks recognized as diverging * fix broken test with diverging function * Allow empty blocks when parsing ir * fix ir test to match generated ir * fmt Cargo.toml * fix formatting * propogate divergence through more expression types * add test for diverging expressions * make some type predicate methods more complete Co-authored-by: Mohammad Fawaz <[email protected]>
8siceVobpu · Sep 14, 2022 · 9aa7b8b · 9aa7b8b
1 parent 64266d3
commit 9aa7b8b
Show file tree

Hide file tree

Showing 28 changed files with 1,324 additions and 455 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/Cargo.toml b/Cargo.toml
@@ -14,6 +14,7 @@ members = [
     "sway-ast",
     "sway-core",
     "sway-ir",
+    "sway-ir/sway-ir-macros",
     "sway-lsp",
     "sway-parse",
     "sway-types",

diff --git a/sway-core/src/ir_generation/function.rs b/sway-core/src/ir_generation/function.rs
diff --git a/sway-core/src/semantic_analysis/ast_node/code_block.rs b/sway-core/src/semantic_analysis/ast_node/code_block.rs
@@ -47,33 +47,39 @@ impl TypedCodeBlock {
                 _ => None,
             })
             .flatten();
+        let span = implicit_return_span.unwrap_or_else(|| code_block.whole_block_span.clone());
 
         // find the implicit return, if any, and use it as the code block's return type.
         // The fact that there is at most one implicit return is an invariant held by the parser.
-        let return_type = evaluated_contents.iter().find_map(|x| match x {
-            TypedAstNode {
-                content:
-                    TypedAstNodeContent::ImplicitReturnExpression(TypedExpression {
-                        ref return_type,
-                        ..
-                    }),
-                ..
-            } if !x.deterministically_aborts() => Some(*return_type),
-            _ => None,
-        });
+        // If any node diverges then the entire block has unknown type.
+        let block_type = evaluated_contents
+            .iter()
+            .find_map(|node| {
+                if node.deterministically_aborts() {
+                    Some(insert_type(TypeInfo::Unknown))
+                } else {
+                    match node {
+                        TypedAstNode {
+                            content:
+                                TypedAstNodeContent::ImplicitReturnExpression(TypedExpression {
+                                    ref return_type,
+                                    ..
+                                }),
+                            ..
+                        } => Some(*return_type),
+                        _ => None,
+                    }
+                }
+            })
+            .unwrap_or_else(|| insert_type(TypeInfo::Tuple(Vec::new())));
 
-        if let Some(return_type) = return_type {
-            let span = implicit_return_span.unwrap_or_else(|| code_block.whole_block_span.clone());
-            let (mut new_warnings, new_errors) = ctx.unify_with_self(return_type, &span);
-            warnings.append(&mut new_warnings);
-            errors.append(&mut new_errors.into_iter().map(|x| x.into()).collect());
-            // The annotation will result in a cast, so set the return type accordingly.
-        }
+        let (new_warnings, new_errors) = ctx.unify_with_self(block_type, &span);
+        warnings.extend(new_warnings);
+        errors.extend(new_errors.into_iter().map(|type_error| type_error.into()));
 
         let typed_code_block = TypedCodeBlock {
             contents: evaluated_contents,
         };
-        let type_id = return_type.unwrap_or_else(|| insert_type(TypeInfo::Tuple(Vec::new())));
-        ok((typed_code_block, type_id), warnings, errors)
+        ok((typed_code_block, block_type), warnings, errors)
     }
 }
diff --git a/...re/src/semantic_analysis/ast_node/expression/match_expression/typed/typed_match_branch.rs b/...re/src/semantic_analysis/ast_node/expression/match_expression/typed/typed_match_branch.rs
@@ -115,7 +115,7 @@ impl TypedMatchBranch {
             }
             typed_result_expression_variant => {
                 code_block_contents.push(TypedAstNode {
-                    content: TypedAstNodeContent::Expression(TypedExpression {
+                    content: TypedAstNodeContent::ImplicitReturnExpression(TypedExpression {
                         expression: typed_result_expression_variant,
                         return_type: typed_result_return_type,
                         is_constant: typed_result_is_constant,

diff --git a/sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs b/sway-core/src/semantic_analysis/ast_node/expression/typed_expression.rs
@@ -760,14 +760,11 @@ impl TypedExpression {
         let mut warnings = res.warnings;
         let mut errors = res.errors;
 
-        // if one of the expressions deterministically aborts, we don't want to type check it.
-        if !typed_expression.deterministically_aborts() {
-            // if the return type cannot be cast into the annotation type then it is a type error
-            let (mut new_warnings, new_errors) =
-                ctx.unify_with_self(typed_expression.return_type, &expr_span);
-            warnings.append(&mut new_warnings);
-            errors.append(&mut new_errors.into_iter().map(|x| x.into()).collect());
-        }
+        // if the return type cannot be cast into the annotation type then it is a type error
+        let (mut new_warnings, new_errors) =
+            ctx.unify_with_self(typed_expression.return_type, &expr_span);
+        warnings.append(&mut new_warnings);
+        errors.append(&mut new_errors.into_iter().map(|x| x.into()).collect());
 
         // The annotation may result in a cast, which is handled in the type engine.
         typed_expression.return_type = check!(
@@ -1155,17 +1152,25 @@ impl TypedExpression {
             .clone()
             .map(|x| x.1)
             .unwrap_or_else(|| asm.whole_block_span.clone());
-        let return_type = check!(
-            ctx.resolve_type_with_self(
-                insert_type(asm.return_type.clone()),
-                &asm_span,
-                EnforceTypeArguments::No,
-                None
-            ),
-            insert_type(TypeInfo::ErrorRecovery),
-            warnings,
-            errors,
-        );
+        let diverges = asm
+            .body
+            .iter()
+            .any(|asm_op| matches!(asm_op.op_name.as_str(), "rvrt" | "ret"));
+        let return_type = if diverges {
+            insert_type(TypeInfo::Unknown)
+        } else {
+            check!(
+                ctx.resolve_type_with_self(
+                    insert_type(asm.return_type.clone()),
+                    &asm_span,
+                    EnforceTypeArguments::No,
+                    None
+                ),
+                insert_type(TypeInfo::ErrorRecovery),
+                warnings,
+                errors,
+            )
+        };
         // type check the initializers
         let typed_registers = asm
             .registers

diff --git a/sway-core/src/semantic_analysis/ast_node/mod.rs b/sway-core/src/semantic_analysis/ast_node/mod.rs
@@ -505,24 +505,19 @@ impl TypedAstNode {
         };
 
         if let TypedAstNode {
-            content: TypedAstNodeContent::Expression(TypedExpression { ref expression, .. }),
+            content: TypedAstNodeContent::Expression(TypedExpression { .. }),
             ..
         } = node
         {
-            if !matches!(
-                expression,
-                TypedExpressionVariant::Break | TypedExpressionVariant::Continue,
-            ) {
-                let warning = Warning::UnusedReturnValue {
-                    r#type: Box::new(node.type_info()),
-                };
-                assert_or_warn!(
-                    node.type_info().is_unit() || node.type_info() == TypeInfo::ErrorRecovery,
-                    warnings,
-                    node.span.clone(),
-                    warning
-                );
-            }
+            let warning = Warning::UnusedReturnValue {
+                r#type: Box::new(node.type_info()),
+            };
+            assert_or_warn!(
+                node.type_info().can_safely_ignore(),
+                warnings,
+                node.span.clone(),
+                warning
+            );
         }
 
         ok(node, warnings, errors)

diff --git a/sway-core/src/type_system/type_info.rs b/sway-core/src/type_system/type_info.rs
@@ -558,6 +558,10 @@ impl TypeInfo {
             TypeInfo::Tuple(fields) => fields
                 .iter()
                 .any(|field_type| look_up_type_id(field_type.type_id).is_uninhabited()),
+            TypeInfo::Ref(type_id, _) => look_up_type_id(*type_id).is_uninhabited(),
+            TypeInfo::Array(type_id, size, _) => {
+                *size > 0 && look_up_type_id(*type_id).is_uninhabited()
+            }
             _ => false,
         }
     }
@@ -605,6 +609,28 @@ impl TypeInfo {
                 }
                 all_zero_sized
             }
+            TypeInfo::Ref(type_id, _) => look_up_type_id(*type_id).is_zero_sized(),
+            TypeInfo::Array(type_id, size, _) => {
+                *size == 0 || look_up_type_id(*type_id).is_zero_sized()
+            }
+            _ => false,
+        }
+    }
+
+    pub fn can_safely_ignore(&self) -> bool {
+        if self.is_zero_sized() {
+            return true;
+        }
+        match self {
+            TypeInfo::Tuple(fields) => fields
+                .iter()
+                .all(|type_argument| look_up_type_id(type_argument.type_id).can_safely_ignore()),
+            TypeInfo::Array(type_id, size, _) => {
+                *size == 0 || look_up_type_id(*type_id).can_safely_ignore()
+            }
+            TypeInfo::Ref(type_id, _) => look_up_type_id(*type_id).can_safely_ignore(),
+            TypeInfo::ErrorRecovery => true,
+            TypeInfo::Unknown => true,
             _ => false,
         }
     }

diff --git a/sway-ir/Cargo.toml b/sway-ir/Cargo.toml
@@ -13,4 +13,5 @@ anyhow = "1.0"
 filecheck = "0.5"
 generational-arena = "0.2"
 peg = "0.7"
+sway-ir-macros = { version = "0.24.3", path = "sway-ir-macros" }
 sway-types = { version = "0.24.3", path = "../sway-types" }
diff --git a/sway-ir/src/asm.rs b/sway-ir/src/asm.rs
@@ -16,15 +16,17 @@
 
 use sway_types::ident::Ident;
 
-use crate::{context::Context, irtype::Type, metadata::MetadataIndex, value::Value};
+use crate::{
+    context::Context, irtype::Type, metadata::MetadataIndex, pretty::DebugWithContext, value::Value,
+};
 
 /// A wrapper around an [ECS](https://github.com/fitzgen/generational-arena) handle into the
 /// [`Context`].
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
-pub struct AsmBlock(pub generational_arena::Index);
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, DebugWithContext)]
+pub struct AsmBlock(#[in_context(asm_blocks)] pub generational_arena::Index);
 
 #[doc(hidden)]
-#[derive(Clone, Debug)]
+#[derive(Clone, Debug, DebugWithContext)]
 pub struct AsmBlockContent {
     pub args_names: Vec<Ident>,
     pub body: Vec<AsmInstruction>,
@@ -65,8 +67,16 @@ impl AsmBlock {
     }
 
     /// Return the [`AsmBlock`] return type.
-    pub fn get_type(&self, context: &Context) -> Option<Type> {
+    pub fn get_type(&self, context: &Context) -> Type {
         // The type is a named register, which will be a u64.
-        Some(context.asm_blocks[self.0].return_type)
+        context.asm_blocks[self.0].return_type
+    }
+
+    pub fn is_diverging(&self, context: &Context) -> bool {
+        let content = &context.asm_blocks[self.0];
+        content
+            .body
+            .iter()
+            .any(|asm_instruction| matches!(asm_instruction.name.as_str(), "rvrt" | "ret"))
     }
 }
diff --git a/sway-ir/src/block.rs b/sway-ir/src/block.rs
@@ -15,12 +15,13 @@ use crate::{
     error::IrError,
     function::Function,
     instruction::{Instruction, InstructionInserter, InstructionIterator},
+    pretty::DebugWithContext,
     value::{Value, ValueDatum},
 };
 
 /// A wrapper around an [ECS](https://github.com/fitzgen/generational-arena) handle into the
 /// [`Context`].
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, DebugWithContext)]
 pub struct Block(pub generational_arena::Index);
 
 #[doc(hidden)]

diff --git a/sway-ir/src/constant.rs b/sway-ir/src/constant.rs
@@ -3,18 +3,19 @@
 use crate::{
     context::Context,
     irtype::{Aggregate, Type},
+    pretty::DebugWithContext,
     value::Value,
 };
 
 /// A [`Type`] and constant value, including [`ConstantValue::Undef`] for uninitialized constants.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, DebugWithContext)]
 pub struct Constant {
     pub ty: Type,
     pub value: ConstantValue,
 }
 
 /// A constant representation of each of the supported [`Type`]s.
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, DebugWithContext)]
 pub enum ConstantValue {
     Undef,
     Unit,

diff --git a/sway-ir/src/instruction.rs b/sway-ir/src/instruction.rs
@@ -17,10 +17,11 @@ use crate::{
     function::Function,
     irtype::{Aggregate, Type},
     pointer::Pointer,
+    pretty::DebugWithContext,
     value::{Value, ValueDatum},
 };
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, DebugWithContext)]
 pub enum Instruction {
     /// Address of a non-copy (memory) value
     AddrOf(Value),
@@ -179,7 +180,7 @@ impl Instruction {
     pub fn get_type(&self, context: &Context) -> Option<Type> {
         match self {
             Instruction::AddrOf(_) => Some(Type::Uint(64)),
-            Instruction::AsmBlock(asm_block, _) => asm_block.get_type(context),
+            Instruction::AsmBlock(asm_block, _) => Some(asm_block.get_type(context)),
             Instruction::BitCast(_, ty) => Some(*ty),
             Instruction::Call(function, _) => Some(context.functions[function.0].return_type),
             Instruction::Cmp(..) => Some(Type::Bool),

diff --git a/sway-ir/src/irtype.rs b/sway-ir/src/irtype.rs
@@ -9,9 +9,9 @@
 //! [`Aggregate`] is an abstract collection of [`Type`]s used for structs, unions and arrays,
 //! though see below for future improvements around splitting arrays into a different construct.
 
-use crate::{context::Context, Pointer};
+use crate::{context::Context, pretty::DebugWithContext, Pointer};
 
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone, Copy, DebugWithContext)]
 pub enum Type {
     Unit,
     Bool,
@@ -131,11 +131,11 @@ impl Type {
 /// that they represent the same collection of types.  Instead the `is_equivalent()` method is
 /// provided.  XXX Perhaps `Hash` should be impl'd directly without `Eq` if possible?
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash)]
-pub struct Aggregate(pub generational_arena::Index);
+#[derive(Clone, Copy, Debug, Eq, PartialEq, Hash, DebugWithContext)]
+pub struct Aggregate(#[in_context(aggregates)] pub generational_arena::Index);
 
 #[doc(hidden)]
-#[derive(Debug, Clone)]
+#[derive(Debug, Clone, DebugWithContext)]
 pub enum AggregateContent {
     ArrayType(Type, u64),
     FieldTypes(Vec<Type>),

diff --git a/sway-ir/src/lib.rs b/sway-ir/src/lib.rs
@@ -63,6 +63,8 @@ pub mod parser;
 pub use parser::*;
 pub mod pointer;
 pub use pointer::*;
+pub mod pretty;
+pub use pretty::*;
 pub mod printer;
 pub use printer::*;
 pub mod value;

diff --git a/sway-ir/src/parser.rs b/sway-ir/src/parser.rs
@@ -90,7 +90,7 @@ mod ir_builder {
                 }
 
             rule block_decl() -> IrAstBlock
-                = label:id() ":" _ instructions: instr_decl()+ {
+                = label:id() ":" _ instructions: instr_decl()* {
                     IrAstBlock {
                         label,
                         instructions