From 9ebe3190429e439ed7ff657f96a28499b31e41a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez=20Calder=C3=B3n?= Date: Thu, 11 Apr 2024 18:02:05 -0300 Subject: [PATCH 1/5] Fix bug --- crates/concrete_ir/src/lowering.rs | 38 +++++++++--------------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/crates/concrete_ir/src/lowering.rs b/crates/concrete_ir/src/lowering.rs index 7663f7f..98357cc 100644 --- a/crates/concrete_ir/src/lowering.rs +++ b/crates/concrete_ir/src/lowering.rs @@ -428,25 +428,17 @@ fn lower_while(builder: &mut FnBodyBuilder, info: &WhileStmt) -> Result<(), Lowe )?; } - // keet idx to change terminator if there is no return - let last_then_block_idx = if !matches!( - builder.body.basic_blocks.last().unwrap().terminator.kind, - TerminatorKind::Return - ) { - builder.body.basic_blocks.len(); - let statements = std::mem::take(&mut builder.statements); - let idx = builder.body.basic_blocks.len(); - builder.body.basic_blocks.push(BasicBlock { - statements, - terminator: Box::new(Terminator { - span: None, - kind: TerminatorKind::Unreachable, - }), - }); - Some(idx) - } else { - None - }; + builder.body.basic_blocks.len(); + let statements = std::mem::take(&mut builder.statements); + builder.body.basic_blocks.push(BasicBlock { + statements, + terminator: Box::new(Terminator { + span: None, + kind: TerminatorKind::Goto { + target: check_block_idx, + }, + }), + }); let otherwise_block_idx = builder.body.basic_blocks.len(); @@ -461,14 +453,6 @@ fn lower_while(builder: &mut FnBodyBuilder, info: &WhileStmt) -> Result<(), Lowe }; builder.body.basic_blocks[check_block_idx].terminator.kind = kind; - if let Some(last_then_block_idx) = last_then_block_idx { - builder.body.basic_blocks[last_then_block_idx] - .terminator - .kind = TerminatorKind::Goto { - target: check_block_idx, - }; - } - Ok(()) } From ef166fadc97d71675c3aab3c8e48e5cbd8435c45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez=20Calder=C3=B3n?= Date: Fri, 12 Apr 2024 16:10:19 -0300 Subject: [PATCH 2/5] Add assert to enforce rule --- crates/concrete_ir/src/lowering.rs | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/crates/concrete_ir/src/lowering.rs b/crates/concrete_ir/src/lowering.rs index 98357cc..acb8693 100644 --- a/crates/concrete_ir/src/lowering.rs +++ b/crates/concrete_ir/src/lowering.rs @@ -366,12 +366,18 @@ fn lower_statement( statements::Statement::Assign(info) => lower_assign(builder, info)?, statements::Statement::Match(_) => todo!(), statements::Statement::For(_) => todo!(), - statements::Statement::If(info) => lower_if_statement(builder, info)?, + statements::Statement::If(info) => { + lower_if_statement(builder, info)?; + assert!(builder.statements.is_empty()); + } statements::Statement::Let(info) => lower_let(builder, info)?, statements::Statement::Return(info) => { lower_return(builder, info, ret_type)?; } - statements::Statement::While(info) => lower_while(builder, info)?, + statements::Statement::While(info) => { + lower_while(builder, info)?; + assert!(builder.statements.is_empty()); + } statements::Statement::FnCall(info) => { lower_fn_call(builder, info)?; } From c4058199ed328d0e9ccd11323a43b8586e4516e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez=20Calder=C3=B3n?= Date: Fri, 12 Apr 2024 16:52:19 -0300 Subject: [PATCH 3/5] Add test for while_if_false.con --- crates/concrete_driver/tests/examples.rs | 1 + examples/while_if_false.con | 14 ++++++++++++++ 2 files changed, 15 insertions(+) create mode 100644 examples/while_if_false.con diff --git a/crates/concrete_driver/tests/examples.rs b/crates/concrete_driver/tests/examples.rs index 0154563..d5cfba6 100644 --- a/crates/concrete_driver/tests/examples.rs +++ b/crates/concrete_driver/tests/examples.rs @@ -16,6 +16,7 @@ mod common; #[test_case(include_str!("../../../examples/structs.con"), "structs", false, 8 ; "structs.con")] #[test_case(include_str!("../../../examples/casts.con"), "casts", false, 2 ; "casts.con")] #[test_case(include_str!("../../../examples/malloc.con"), "malloc", false, 5 ; "malloc.con")] +#[test_case(include_str!("../../../examples/while_if_false.con"), "while_if_false", false, 7 ; "while_if_false.con")] fn example_tests(source: &str, name: &str, is_library: bool, status_code: i32) { assert_eq!( status_code, diff --git a/examples/while_if_false.con b/examples/while_if_false.con new file mode 100644 index 0000000..46f1ad5 --- /dev/null +++ b/examples/while_if_false.con @@ -0,0 +1,14 @@ +mod Example { + fn main() -> i32 { + let mut result: i32 = 7; + + while false { + if false { + return 0; + } + result = 14; + } + + return result; + } +} From abcd93e5aae70fc6f4cee982a1e8c8ec12d6b006 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez=20Calder=C3=B3n?= Date: Fri, 12 Apr 2024 18:00:01 -0300 Subject: [PATCH 4/5] Add if_if_false.con example --- crates/concrete_driver/tests/examples.rs | 1 + examples/if_if_false.con | 15 +++++++++++++++ 2 files changed, 16 insertions(+) create mode 100644 examples/if_if_false.con diff --git a/crates/concrete_driver/tests/examples.rs b/crates/concrete_driver/tests/examples.rs index d5cfba6..f26846a 100644 --- a/crates/concrete_driver/tests/examples.rs +++ b/crates/concrete_driver/tests/examples.rs @@ -17,6 +17,7 @@ mod common; #[test_case(include_str!("../../../examples/casts.con"), "casts", false, 2 ; "casts.con")] #[test_case(include_str!("../../../examples/malloc.con"), "malloc", false, 5 ; "malloc.con")] #[test_case(include_str!("../../../examples/while_if_false.con"), "while_if_false", false, 7 ; "while_if_false.con")] +#[test_case(include_str!("../../../examples/if_if_false.con"), "if_if_false", false, 7 ; "if_if_false.con")] fn example_tests(source: &str, name: &str, is_library: bool, status_code: i32) { assert_eq!( status_code, diff --git a/examples/if_if_false.con b/examples/if_if_false.con new file mode 100644 index 0000000..a38c67d --- /dev/null +++ b/examples/if_if_false.con @@ -0,0 +1,15 @@ +mod Example { + fn main() -> i32 { + let foo: i32 = 7; + + if true { + if false { + return 1; + } + } else { + foo = 14; + } + + return foo; + } +} From 13b5bb374bc63fe2b6f612ac7ab915e690cf0793 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Juli=C3=A1n=20Gonz=C3=A1lez=20Calder=C3=B3n?= Date: Fri, 12 Apr 2024 18:11:11 -0300 Subject: [PATCH 5/5] Fix if bug by always adding terminator block --- crates/concrete_ir/src/lowering.rs | 66 +++++++++--------------------- 1 file changed, 19 insertions(+), 47 deletions(-) diff --git a/crates/concrete_ir/src/lowering.rs b/crates/concrete_ir/src/lowering.rs index acb8693..ba54eeb 100644 --- a/crates/concrete_ir/src/lowering.rs +++ b/crates/concrete_ir/src/lowering.rs @@ -309,16 +309,14 @@ fn lower_func( lower_statement(&mut builder, stmt, ret_type.clone())?; } - if !builder.statements.is_empty() { - let statements = std::mem::take(&mut builder.statements); - builder.body.basic_blocks.push(BasicBlock { - statements, - terminator: Box::new(Terminator { - span: None, - kind: TerminatorKind::Return, - }), - }); - } + let statements = std::mem::take(&mut builder.statements); + builder.body.basic_blocks.push(BasicBlock { + statements, + terminator: Box::new(Terminator { + span: None, + kind: TerminatorKind::Return, + }), + }); let (mut ctx, body) = (builder.ctx, builder.body); ctx.unresolved_function_signatures.remove(&body.id); @@ -501,13 +499,9 @@ fn lower_if_statement(builder: &mut FnBodyBuilder, info: &IfExpr) -> Result<(), } // keet idx to change terminator - let last_then_block_idx = if !matches!( - builder.body.basic_blocks.last().unwrap().terminator.kind, - TerminatorKind::Return - ) { + let last_then_block_idx = { builder.body.basic_blocks.len(); let statements = std::mem::take(&mut builder.statements); - let idx = builder.body.basic_blocks.len(); builder.body.basic_blocks.push(BasicBlock { statements, terminator: Box::new(Terminator { @@ -515,9 +509,7 @@ fn lower_if_statement(builder: &mut FnBodyBuilder, info: &IfExpr) -> Result<(), kind: TerminatorKind::Unreachable, }), }); - Some(idx) - } else { - None + builder.body.basic_blocks.len() - 1 }; let first_else_block_idx = builder.body.basic_blocks.len(); @@ -530,32 +522,23 @@ fn lower_if_statement(builder: &mut FnBodyBuilder, info: &IfExpr) -> Result<(), builder.body.locals[builder.ret_local].ty.clone(), )?; } - } - let last_else_block_idx = if !matches!( - builder.body.basic_blocks.last().unwrap().terminator.kind, - TerminatorKind::Return - ) { - builder.body.basic_blocks.len(); let statements = std::mem::take(&mut builder.statements); - let idx = builder.body.basic_blocks.len(); builder.body.basic_blocks.push(BasicBlock { statements, terminator: Box::new(Terminator { span: None, - kind: TerminatorKind::Unreachable, + kind: TerminatorKind::Goto { + target: builder.body.basic_blocks.len() + 1, + }, }), }); - Some(idx) - } else { - None - }; + } let targets = SwitchTargets { values: vec![discriminator_type.kind.get_falsy_value()], targets: vec![first_else_block_idx, first_then_block_idx], }; - let kind = TerminatorKind::SwitchInt { discriminator: Operand::Place(place), targets, @@ -564,22 +547,11 @@ fn lower_if_statement(builder: &mut FnBodyBuilder, info: &IfExpr) -> Result<(), let next_block_idx = builder.body.basic_blocks.len(); - // check if the - if let Some(last_then_block_idx) = last_then_block_idx { - builder.body.basic_blocks[last_then_block_idx] - .terminator - .kind = TerminatorKind::Goto { - target: next_block_idx, - }; - } - - if let Some(last_else_block_idx) = last_else_block_idx { - builder.body.basic_blocks[last_else_block_idx] - .terminator - .kind = TerminatorKind::Goto { - target: next_block_idx, - }; - } + builder.body.basic_blocks[last_then_block_idx] + .terminator + .kind = TerminatorKind::Goto { + target: next_block_idx, + }; Ok(()) }