Skip to content

Commit

Permalink
Do not progress with IR generation if deterministically_aborts (Fue…
Browse files Browse the repository at this point in the history
…lLabs#3460)

Fixes FuelLabs#3303 

The existing check for `deterministically_aborts` assumes that function
applications are inlined. This shouldn't be so for the newly added call
to `determinisitically_aborts`. So I've added a new parameter to the
`DeterministicallyAborts` trait to switch considering "calls are
inlined" or not semantics. I'm not sure if this is the best way, so I'm
open to suggestions.
  • Loading branch information
vaivaswatha authored Nov 29, 2022
1 parent 34affe1 commit 52bcfc7
Show file tree
Hide file tree
Showing 10 changed files with 71 additions and 36 deletions.
4 changes: 3 additions & 1 deletion sway-core/src/ir_generation/function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use crate::{
},
metadata::MetadataManager,
type_system::{LogId, TypeId, TypeInfo},
types::DeterministicallyAborts,
PartialEqWithTypeEngine, TypeEngine,
};
use declaration_engine::de_get_function;
Expand Down Expand Up @@ -1487,8 +1488,9 @@ impl<'te> FnCompiler<'te> {

// We must compile the RHS before checking for shadowing, as it will still be in the
// previous scope.
let body_deterministically_aborts = body.deterministically_aborts(false);
let init_val = self.compile_expression(context, md_mgr, body)?;
if init_val.is_diverging(context) {
if init_val.is_diverging(context) || body_deterministically_aborts {
return Ok(Some(init_val));
}
let local_name = self.lexical_map.insert(name.as_str().to_owned());
Expand Down
6 changes: 4 additions & 2 deletions sway-core/src/language/ty/ast_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -98,11 +98,13 @@ impl CollectTypesMetadata for TyAstNode {
}

impl DeterministicallyAborts for TyAstNode {
fn deterministically_aborts(&self) -> bool {
fn deterministically_aborts(&self, check_call_body: bool) -> bool {
use TyAstNodeContent::*;
match &self.content {
Declaration(_) => false,
Expression(exp) | ImplicitReturnExpression(exp) => exp.deterministically_aborts(),
Expression(exp) | ImplicitReturnExpression(exp) => {
exp.deterministically_aborts(check_call_body)
}
SideEffect => false,
}
}
Expand Down
6 changes: 4 additions & 2 deletions sway-core/src/language/ty/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,9 @@ impl ReplaceDecls for TyCodeBlock {
}

impl DeterministicallyAborts for TyCodeBlock {
fn deterministically_aborts(&self) -> bool {
self.contents.iter().any(|x| x.deterministically_aborts())
fn deterministically_aborts(&self, check_call_body: bool) -> bool {
self.contents
.iter()
.any(|x| x.deterministically_aborts(check_call_body))
}
}
61 changes: 37 additions & 24 deletions sway-core/src/language/ty/expression/expression.rs
Original file line number Diff line number Diff line change
Expand Up @@ -394,47 +394,57 @@ impl CollectTypesMetadata for TyExpression {
}

impl DeterministicallyAborts for TyExpression {
fn deterministically_aborts(&self) -> bool {
fn deterministically_aborts(&self, check_call_body: bool) -> bool {
use TyExpressionVariant::*;
match &self.expression {
FunctionApplication {
function_decl_id,
arguments,
..
} => {
if !check_call_body {
return false;
}
let function_decl = match de_get_function(function_decl_id.clone(), &self.span) {
Ok(decl) => decl,
Err(_e) => panic!("failed to get function"),
};
function_decl.body.deterministically_aborts()
|| arguments.iter().any(|(_, x)| x.deterministically_aborts())
}
Tuple { fields, .. } => fields.iter().any(|x| x.deterministically_aborts()),
Array { contents, .. } => contents.iter().any(|x| x.deterministically_aborts()),
CodeBlock(contents) => contents.deterministically_aborts(),
LazyOperator { lhs, .. } => lhs.deterministically_aborts(),
StructExpression { fields, .. } => {
fields.iter().any(|x| x.value.deterministically_aborts())
function_decl.body.deterministically_aborts(check_call_body)
|| arguments
.iter()
.any(|(_, x)| x.deterministically_aborts(check_call_body))
}
Tuple { fields, .. } => fields
.iter()
.any(|x| x.deterministically_aborts(check_call_body)),
Array { contents, .. } => contents
.iter()
.any(|x| x.deterministically_aborts(check_call_body)),
CodeBlock(contents) => contents.deterministically_aborts(check_call_body),
LazyOperator { lhs, .. } => lhs.deterministically_aborts(check_call_body),
StructExpression { fields, .. } => fields
.iter()
.any(|x| x.value.deterministically_aborts(check_call_body)),
EnumInstantiation { contents, .. } => contents
.as_ref()
.map(|x| x.deterministically_aborts())
.map(|x| x.deterministically_aborts(check_call_body))
.unwrap_or(false),
AbiCast { address, .. } => address.deterministically_aborts(),
AbiCast { address, .. } => address.deterministically_aborts(check_call_body),
StructFieldAccess { .. }
| Literal(_)
| StorageAccess { .. }
| VariableExpression { .. }
| FunctionParameter
| TupleElemAccess { .. } => false,
IntrinsicFunction(kind) => kind.deterministically_aborts(),
IntrinsicFunction(kind) => kind.deterministically_aborts(check_call_body),
ArrayIndex { prefix, index } => {
prefix.deterministically_aborts() || index.deterministically_aborts()
prefix.deterministically_aborts(check_call_body)
|| index.deterministically_aborts(check_call_body)
}
AsmExpression { registers, .. } => registers.iter().any(|x| {
x.initializer
.as_ref()
.map(|x| x.deterministically_aborts())
.map(|x| x.deterministically_aborts(check_call_body))
.unwrap_or(false)
}),
IfExp {
Expand All @@ -443,25 +453,28 @@ impl DeterministicallyAborts for TyExpression {
r#else,
..
} => {
condition.deterministically_aborts()
|| (then.deterministically_aborts()
condition.deterministically_aborts(check_call_body)
|| (then.deterministically_aborts(check_call_body)
&& r#else
.as_ref()
.map(|x| x.deterministically_aborts())
.map(|x| x.deterministically_aborts(check_call_body))
.unwrap_or(false))
}
AbiName(_) => false,
EnumTag { exp } => exp.deterministically_aborts(),
UnsafeDowncast { exp, .. } => exp.deterministically_aborts(),
EnumTag { exp } => exp.deterministically_aborts(check_call_body),
UnsafeDowncast { exp, .. } => exp.deterministically_aborts(check_call_body),
WhileLoop { condition, body } => {
condition.deterministically_aborts() || body.deterministically_aborts()
condition.deterministically_aborts(check_call_body)
|| body.deterministically_aborts(check_call_body)
}
Break => false,
Continue => false,
Reassignment(reassignment) => reassignment.rhs.deterministically_aborts(),
StorageReassignment(storage_reassignment) => {
storage_reassignment.rhs.deterministically_aborts()
Reassignment(reassignment) => {
reassignment.rhs.deterministically_aborts(check_call_body)
}
StorageReassignment(storage_reassignment) => storage_reassignment
.rhs
.deterministically_aborts(check_call_body),
// TODO: Is this correct?
// I'm not sure what this function is supposed to do exactly. It's called
// "deterministically_aborts" which I thought meant it checks for an abort/panic, but
Expand Down
7 changes: 5 additions & 2 deletions sway-core/src/language/ty/expression/intrinsic_function.rs
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,12 @@ impl DisplayWithTypeEngine for TyIntrinsicFunctionKind {
}

impl DeterministicallyAborts for TyIntrinsicFunctionKind {
fn deterministically_aborts(&self) -> bool {
fn deterministically_aborts(&self, check_call_body: bool) -> bool {
matches!(self.kind, Intrinsic::Revert)
|| self.arguments.iter().any(|x| x.deterministically_aborts())
|| self
.arguments
.iter()
.any(|x| x.deterministically_aborts(check_call_body))
}
}

Expand Down
2 changes: 1 addition & 1 deletion sway-core/src/semantic_analysis/ast_node/code_block.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ impl ty::TyCodeBlock {
let block_type = evaluated_contents
.iter()
.find_map(|node| {
if node.deterministically_aborts() {
if node.deterministically_aborts(true) {
Some(ctx.type_engine.insert_type(TypeInfo::Unknown))
} else {
match node {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ impl ty::TyMatchBranch {
};

// unify the return type from the typed result with the type annotation
if !typed_result.deterministically_aborts() {
if !typed_result.deterministically_aborts(true) {
append!(
ctx.unify_with_self(typed_result.return_type, &typed_result.span),
warnings,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ pub(crate) fn instantiate_if_expression(
let mut errors = vec![];

// if the branch aborts, then its return type doesn't matter.
let then_deterministically_aborts = then.deterministically_aborts();
let then_deterministically_aborts = then.deterministically_aborts(true);
if !then_deterministically_aborts {
// if this does not deterministically_abort, check the block return type
let ty_to_check = if r#else.is_some() {
Expand All @@ -38,7 +38,7 @@ pub(crate) fn instantiate_if_expression(
}
let mut else_deterministically_aborts = false;
let r#else = r#else.map(|r#else| {
else_deterministically_aborts = r#else.deterministically_aborts();
else_deterministically_aborts = r#else.deterministically_aborts(true);
let ty_to_check = if then_deterministically_aborts {
type_annotation
} else {
Expand Down
3 changes: 2 additions & 1 deletion sway-core/src/types/deterministically_aborts.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/// If this expression deterministically_aborts 100% of the time, this function returns
/// `true`. Used in dead-code and control-flow analysis.
/// if `check_call_body` is set, body of the callee is inspected at call sites.
pub(crate) trait DeterministicallyAborts {
fn deterministically_aborts(&self) -> bool;
fn deterministically_aborts(&self, check_call_body: bool) -> bool;
}
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,16 @@ fn diverge_in_reassignment() -> u64 {
123
}

fn diverge_with_if_else(b: bool) -> u64 {
let x: u64 = if b {
return 5;
} else {
return 1;
};

return x;
}

fn main() -> u64 {
assert(5 == diverge_in_let_body());
assert(5 == diverge_in_struct_0());
Expand All @@ -244,6 +254,8 @@ fn main() -> u64 {
assert(5 == diverge_in_func_arg());
assert(5 == diverge_in_array_index_array());
assert(5 == diverge_in_array_index_index());
assert(5 == diverge_with_if_else(true));
assert(1 == diverge_with_if_else(false));

// Disabled due to https://github.com/FuelLabs/sway/issues/3061
// assert(5 == diverge_in_op_not());
Expand Down

0 comments on commit 52bcfc7

Please sign in to comment.