Skip to content

Commit

Permalink
Added error for nopanic cycles.
Browse files Browse the repository at this point in the history
  • Loading branch information
orizi committed Jan 2, 2024
1 parent 3205b8a commit 42fc7bd
Show file tree
Hide file tree
Showing 7 changed files with 100 additions and 14 deletions.
20 changes: 16 additions & 4 deletions crates/cairo-lang-lowering/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@ use crate::add_withdraw_gas::add_withdraw_gas;
use crate::borrow_check::borrow_check;
use crate::concretize::concretize_lowered;
use crate::destructs::add_destructs;
use crate::diagnostic::LoweringDiagnostic;
use crate::ids::FunctionId;
use crate::diagnostic::{LoweringDiagnostic, LoweringDiagnosticKind};
use crate::graph_algorithms::feedback_set::flag_add_withdraw_gas;
use crate::implicits::lower_implicits;
use crate::inline::{apply_inlining, PrivInlineData};
use crate::lower::{lower_semantic_function, MultiLowering};
Expand Down Expand Up @@ -275,7 +275,7 @@ pub trait LoweringGroup: SemanticGroup + Upcast<dyn SemanticGroup> {

/// Internal query for reorder_statements to cache the function ids that can be moved.
#[salsa::invoke(crate::optimizations::config::priv_movable_function_ids)]
fn priv_movable_function_ids(&self) -> Arc<UnorderedHashSet<FunctionId>>;
fn priv_movable_function_ids(&self) -> Arc<UnorderedHashSet<ids::FunctionId>>;

/// Returns the configuration struct that controls the behavior of the optimization passes.
#[salsa::input]
Expand Down Expand Up @@ -478,7 +478,19 @@ fn function_with_body_lowering_diagnostics(
let mut diagnostics = DiagnosticsBuilder::default();

if let Ok(lowered) = db.function_with_body_lowering(function_id) {
diagnostics.extend(lowered.diagnostics.clone())
diagnostics.extend(lowered.diagnostics.clone());
if flag_add_withdraw_gas(db) && !lowered.signature.panicable && db.in_cycle(function_id)? {
let location = Location {
stable_location: function_id
.base_semantic_function(db)
.stable_location(db.upcast()),
notes: vec![],
};
diagnostics.add(LoweringDiagnostic {
location,
kind: LoweringDiagnosticKind::NoPanicFunctionCycle,
});
}
}

diagnostics.extend(
Expand Down
4 changes: 4 additions & 0 deletions crates/cairo-lang-lowering/src/diagnostic.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,9 @@ impl DiagnosticEntry for LoweringDiagnostic {
"
.into()
}
LoweringDiagnosticKind::NoPanicFunctionCycle => {
"`nopanic` function call cycle.".into()
},
LoweringDiagnosticKind::LiteralError(literal_error) => literal_error.format(db),
LoweringDiagnosticKind::UnsupportedPattern => {
"Inner patterns are not in this context.".into()
Expand Down Expand Up @@ -132,6 +135,7 @@ pub enum LoweringDiagnosticKind {
NonExhaustiveMatchFelt252,
CannotInlineFunctionThatMightCallItself,
MemberPathLoop,
NoPanicFunctionCycle,
LiteralError(LiteralError),
UnsupportedPattern,
}
17 changes: 9 additions & 8 deletions crates/cairo-lang-lowering/src/graph_algorithms/feedback_set.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
use cairo_lang_diagnostics::Maybe;
use cairo_lang_filesystem::flag::Flag;
use cairo_lang_filesystem::ids::FlagId;
use cairo_lang_utils::extract_matches;
use cairo_lang_utils::graph_algos::feedback_set::calc_feedback_set;
use cairo_lang_utils::ordered_hash_set::OrderedHashSet;

Expand All @@ -18,18 +17,20 @@ pub fn function_with_body_feedback_set(
db.priv_function_with_body_feedback_set_of_representative(r)
}

/// Returns the value of the `add_withdraw_gas` flag, or `true` if the flag is not set.
pub fn flag_add_withdraw_gas(db: &dyn LoweringGroup) -> bool {
db.get_flag(FlagId::new(db.upcast(), "add_withdraw_gas"))
.map(|flag| *flag == Flag::AddWithdrawGas(true))
.unwrap_or(true)
}

/// Query implementation of [crate::db::LoweringGroup::needs_withdraw_gas].
pub fn needs_withdraw_gas(
db: &dyn LoweringGroup,
function: ConcreteFunctionWithBodyId,
) -> Maybe<bool> {
if let Some(flag) = db.get_flag(FlagId::new(db.upcast(), "add_withdraw_gas")) {
if !extract_matches!(*flag, Flag::AddWithdrawGas) {
return Ok(false);
}
}

Ok(db.function_with_body_feedback_set(function)?.contains(&function))
Ok(flag_add_withdraw_gas(db)
&& db.function_with_body_feedback_set(function)?.contains(&function))
}

/// Query implementation of
Expand Down
1 change: 1 addition & 0 deletions crates/cairo-lang-lowering/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ cairo_lang_test_utils::test_file_test!(
assignment :"assignment",
call :"call",
constant :"constant",
cycles :"cycles",
literal :"literal",
destruct :"destruct",
enums :"enums",
Expand Down
67 changes: 67 additions & 0 deletions crates/cairo-lang-lowering/src/test_data/cycles
Original file line number Diff line number Diff line change
@@ -0,0 +1,67 @@
//! > Test nopanic function cycle.

//! > test_runner_name
test_function_lowering

//! > function
fn foo(x: felt252) nopanic {
foo(x);
}

//! > function_name
foo

//! > module_code

//! > semantic_diagnostics

//! > lowering_diagnostics
error: `nopanic` function call cycle.
--> lib.cairo:1:1
fn foo(x: felt252) nopanic {
^**************************^

//! > lowering_flat
Parameters: v17: core::RangeCheck, v18: core::gas::GasBuiltin, v0: core::felt252
blk0 (root):
Statements:
(v8: core::gas::BuiltinCosts) <- core::gas::get_builtin_costs()
End:
Match(match core::gas::withdraw_gas_all(v17, v18, v8) {
Option::Some(v19, v20) => blk1,
Option::None(v21, v22) => blk4,
})

blk1:
Statements:
(v23: core::RangeCheck, v24: core::gas::GasBuiltin, v9: core::panics::PanicResult::<((),)>) <- test::foo(v19, v20, v0)
End:
Match(match_enum(v9) {
PanicResult::Ok(v10) => blk2,
PanicResult::Err(v12) => blk3,
})

blk2:
Statements:
(v2: ()) <- struct_construct()
(v14: ((),)) <- struct_construct(v2)
(v15: core::panics::PanicResult::<((),)>) <- PanicResult::Ok(v14)
End:
Return(v23, v24, v15)

blk3:
Statements:
(v16: core::panics::PanicResult::<((),)>) <- PanicResult::Err(v12)
End:
Return(v23, v24, v16)

blk4:
Statements:
(v3: core::array::Array::<core::felt252>) <- core::array::array_new::<core::felt252>()
(v4: core::felt252) <- 375233589013918064796019u
(v6: core::array::Array::<core::felt252>) <- core::array::array_append::<core::felt252>(v3, v4)
(v5: core::panics::Panic) <- struct_construct()
(v7: (core::panics::Panic, core::array::Array::<core::felt252>)) <- struct_construct(v5, v6)
(v13: core::panics::PanicResult::<((),)>) <- PanicResult::Err(v7)
End:
Return(v21, v22, v13)
2 changes: 1 addition & 1 deletion examples/fib_u128_checked.cairo
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
// Calculates fib...
fn fib(a: u128, b: u128, n: u128) -> Option<u128> implicits(RangeCheck) nopanic {
fn fib(a: u128, b: u128, n: u128) -> Option<u128> implicits(RangeCheck) {
// TODO(orizi): Use match on u128 when supported.
match core::integer::u128_to_felt252(n) {
0 => Option::Some(a),
Expand Down
3 changes: 2 additions & 1 deletion tests/bug_samples/issue2172.cairo
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
#[derive(Drop)]
struct Node {
value: felt252,
left: Option<Box<Node>>,
right: Option<Box<Node>>,
}

fn traverse(node: Node) nopanic {
fn traverse(node: Node) {
let Node{value: _, left, right } = node;
match left {
Option::Some(x) => traverse(x.unbox()),
Expand Down

0 comments on commit 42fc7bd

Please sign in to comment.