Sitelet https://github.com/starkware-libs/cairo/commit/439b76f02f24f6f09611c92d8f64cc376fe28b69
Skip to content

Commit 439b76f

Browse files
oriziorizi
committed
fix(lowering): eliminate spurious E3002 alongside E3001 for moved struct members
1 parent 1b47984 commit 439b76f

3 files changed

Lines changed: 126 additions & 58 deletions

File tree

‎crates/cairo-lang-lowering/src/borrow_check/test_data/borrow_check‎

Lines changed: 92 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,7 @@ End:
144144
blk3:
145145
Statements:
146146
End:
147-
Return(v6)
147+
Return(v1)
148148

149149
//! > ==========================================================================
150150

@@ -368,7 +368,7 @@ End:
368368
blk3:
369369
Statements:
370370
End:
371-
Return(v6)
371+
Return(v1)
372372

373373
//! > ==========================================================================
374374

@@ -411,10 +411,10 @@ Statements:
411411
(v3: core::array::Array::<core::felt252>, v2: ()) <- core::array::ArrayImpl::<core::felt252>::append(v0{`__array_builder_macro_result__`}, v1{`'err_code'`})
412412
(v4: core::felt252) <- 6450273
413413
(v6: core::array::Array::<core::felt252>, v5: ()) <- core::array::ArrayImpl::<core::felt252>::append(v3{`b`}, v4{`'bla'`})
414-
(v8: core::panics::Panic) <- struct_construct()
415-
(v9: (core::panics::Panic, core::array::Array::<core::felt252>)) <- struct_construct(v8{`panic(arr)`}, v7{`arr`})
414+
(v7: core::panics::Panic) <- struct_construct()
415+
(v8: (core::panics::Panic, core::array::Array::<core::felt252>)) <- struct_construct(v7{`panic(arr)`}, v3{`arr`})
416416
End:
417-
Panic(v9)
417+
Panic(v8)
418418

419419
//! > ==========================================================================
420420

@@ -458,28 +458,28 @@ blk0 (root):
458458
Statements:
459459
() <- test::use_non_copy(v0{`x`})
460460
End:
461-
Match(match test::do_match_extern(v1{`x`}) {
462-
Option::Some(v2) => blk1,
461+
Match(match test::do_match_extern(v0{`x`}) {
462+
Option::Some(v1) => blk1,
463463
Option::None => blk2,
464464
})
465465

466466
blk1:
467467
Statements:
468-
(v3: core::option::Option::<test::NonCopy>) <- Option::Some(v2{`do_match_extern(x)`})
468+
(v2: core::option::Option::<test::NonCopy>) <- Option::Some(v1{`do_match_extern(x)`})
469469
End:
470-
Goto(blk3, {v3 -> v6})
470+
Goto(blk3, {v2 -> v5})
471471

472472
blk2:
473473
Statements:
474-
(v4: ()) <- struct_construct()
475-
(v5: core::option::Option::<test::NonCopy>) <- Option::None(v4{`do_match_extern(x)`})
474+
(v3: ()) <- struct_construct()
475+
(v4: core::option::Option::<test::NonCopy>) <- Option::None(v3{`do_match_extern(x)`})
476476
End:
477-
Goto(blk3, {v5 -> v6})
477+
Goto(blk3, {v4 -> v5})
478478

479479
blk3:
480480
Statements:
481481
End:
482-
Return(v6)
482+
Return(v5)
483483

484484
//! > ==========================================================================
485485

@@ -528,6 +528,7 @@ Statements:
528528
(v1: core::integer::u32) <- 17
529529
(v2: core::integer::u32, v3: test::NonCopy) <- struct_destructure(v0{`x.a`})
530530
() <- test::use_non_copy(v3{`x.b`})
531+
(v4: test::MyStruct) <- struct_construct(v1{`x`}, v3{`x`})
531532
End:
532533
Return(v4)
533534

@@ -588,6 +589,8 @@ Statements:
588589
(v4: core::array::Array::<core::felt252>, v5: core::integer::u8) <- struct_destructure(v1{`s2.a`})
589590
() <- test::invalidate(v4{`s2.a`})
590591
(v6: ()) <- struct_construct()
592+
(v7: test::MyStruct) <- struct_construct(v2{`s1`}, v3{`s1`})
593+
(v8: test::MyStruct) <- struct_construct(v4{`s2`}, v5{`s2`})
591594
End:
592595
Return(v7, v8, v6)
593596

@@ -661,6 +664,7 @@ End:
661664
blk3:
662665
Statements:
663666
(v12: ()) <- struct_construct()
667+
(v13: test::MyStruct) <- struct_construct(v11{`self`}, v3{`self`})
664668
End:
665669
Return(v13, v12)
666670

@@ -947,6 +951,7 @@ blk0 (root):
947951
Statements:
948952
(v1: core::felt252, v2: test::NonCopy) <- struct_destructure(v0{`x.b`})
949953
() <- test::consume(v2{`x.b`})
954+
(v3: test::MyStruct) <- struct_construct(v1{`x`}, v2{`x`})
950955
End:
951956
Return(v3)
952957

@@ -1119,16 +1124,18 @@ note: variable was previously used here:
11191124
note: Trait has no implementation in context: core::traits::Copy::<test::Wrapper>.
11201125

11211126
error[E3002]: Variable not dropped.
1122-
--> lib.cairo:12:11
1123-
use_x(x); // Second use.
1124-
^
1125-
note: the variable needs to be dropped due to the potential panic here:
1126-
--> lib.cairo:8:8
1127-
if x.inner.is_zero() {
1128-
^^^^^^^^^^^^^^^^^
1127+
--> lib.cairo:9:15
1128+
use_x(x); // First use.
1129+
^
1130+
note: the variable needs to be dropped due to the divergence here:
1131+
--> lib.cairo:8:5-10:5
1132+
if x.inner.is_zero() {
1133+
_____^
1134+
| use_x(x); // First use.
1135+
| }
1136+
|_____^
11291137
note: Trait has no implementation in context: core::traits::Drop::<test::Wrapper>.
11301138
note: Trait has no implementation in context: core::traits::Destruct::<test::Wrapper>.
1131-
note: Trait has no implementation in context: core::traits::PanicDestruct::<test::Wrapper>.
11321139

11331140
//! > lowering
11341141
Parameters: v0: test::Wrapper
@@ -1156,10 +1163,10 @@ End:
11561163

11571164
blk3:
11581165
Statements:
1159-
() <- test::use_x(v6{`x`})
1160-
(v7: ()) <- struct_construct()
1166+
() <- test::use_x(v5{`x`})
1167+
(v6: ()) <- struct_construct()
11611168
End:
1162-
Return(v7)
1169+
Return(v6)
11631170

11641171
//! > ==========================================================================
11651172

@@ -1257,3 +1264,64 @@ Statements:
12571264
(v1: ()) <- struct_construct()
12581265
End:
12591266
Return(v1)
1267+
1268+
//! > ==========================================================================
1269+
1270+
//! > Spurious E3002 alongside E3001 for orphaned inner struct construction (bug repro).
1271+
1272+
//! > test_runner_name
1273+
test_borrow_check
1274+
1275+
//! > function_code
1276+
fn foo(outer: Outer) {
1277+
bar(outer.inner.a);
1278+
consume(outer.b);
1279+
use_outer(outer);
1280+
}
1281+
1282+
//! > function_name
1283+
foo
1284+
1285+
//! > module_code
1286+
extern type NonCopy;
1287+
1288+
struct Inner {
1289+
a: felt252,
1290+
}
1291+
1292+
struct Outer {
1293+
inner: Inner,
1294+
b: NonCopy,
1295+
}
1296+
1297+
extern fn bar(x: felt252) nopanic;
1298+
extern fn consume(x: NonCopy) nopanic;
1299+
extern fn use_outer(x: Outer) nopanic;
1300+
1301+
//! > semantic_diagnostics
1302+
1303+
//! > lowering_diagnostics
1304+
error[E3001]: Variable was previously moved.
1305+
--> lib.cairo:18:15
1306+
use_outer(outer);
1307+
^^^^^
1308+
note: variable was previously used here:
1309+
--> lib.cairo:17:13
1310+
consume(outer.b);
1311+
^^^^^^^
1312+
note: Trait has no implementation in context: core::traits::Copy::<test::NonCopy>.
1313+
1314+
//! > lowering
1315+
Parameters: v0: test::Outer
1316+
blk0 (root):
1317+
Statements:
1318+
(v1: test::Inner, v2: test::NonCopy) <- struct_destructure(v0{`outer.inner.a`})
1319+
(v3: core::felt252) <- struct_destructure(v1{`outer.inner.a`})
1320+
() <- test::bar(v3{`outer.inner.a`})
1321+
() <- test::consume(v2{`outer.b`})
1322+
(v4: test::Inner) <- struct_construct(v3{`outer`})
1323+
(v5: test::Outer) <- struct_construct(v4{`outer`}, v2{`outer`})
1324+
() <- test::use_outer(v5{`outer`})
1325+
(v6: ()) <- struct_construct()
1326+
End:
1327+
Return(v6)

‎crates/cairo-lang-lowering/src/lower/block_builder.rs‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -145,21 +145,22 @@ impl<'db> BlockBuilder<'db> {
145145
}
146146

147147
// If the variable is not copyable, mark it as [MovedVar].
148-
let var = &ctx.variables[var_id];
149-
let copyable = var.info.copyable.clone();
150-
let ty = var.ty;
151-
152-
if let Err(inference_error) = copyable {
148+
if let Err(inference_error) = &ctx.variables[var_id].info.copyable {
149+
let inference_error = inference_error.clone();
153150
self.semantics.mark_as_used(
154151
BlockStructRecomposer { statements: &mut self.statements, ctx, location },
155152
member_path,
156-
MovedVar { ty, inference_error, last_use_location: location },
153+
MovedVar { var_id, inference_error, last_use_location: location },
157154
);
158155
}
159156

160157
Some(VarUsage { var_id, location })
161158
}
162-
Err(AssembleValueError::Moved(MovedVar { ty, inference_error, last_use_location })) => {
159+
Err(AssembleValueError::Moved(MovedVar {
160+
var_id,
161+
inference_error,
162+
last_use_location,
163+
})) => {
163164
// If the variable was already moved, report an error.
164165
let diag_location = location
165166
.long(ctx.db)
@@ -175,8 +176,6 @@ impl<'db> BlockBuilder<'db> {
175176
LoweringDiagnosticKind::VariableMoved { inference_error },
176177
);
177178

178-
// Create and return a dummy variable.
179-
let var_id = ctx.new_var(VarRequest { ty, location });
180179
Some(VarUsage { var_id, location })
181180
}
182181
Err(AssembleValueError::Missing) => None,

‎crates/cairo-lang-lowering/src/lower/refs.rs‎

Lines changed: 26 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,12 +4,13 @@ use cairo_lang_semantic::expr::fmt::ExprFormatter;
44
use cairo_lang_semantic::expr::inference::InferenceError;
55
use cairo_lang_semantic::items::structure::StructSemantic;
66
use cairo_lang_semantic::usage::MemberPath;
7-
use cairo_lang_semantic::{self as semantic, ConcreteTypeId, TypeLongId};
7+
use cairo_lang_semantic::{self as semantic};
88
use cairo_lang_utils::ordered_hash_map::OrderedHashMap;
9-
use cairo_lang_utils::{Intern, extract_matches, try_extract_matches};
9+
use cairo_lang_utils::{extract_matches, try_extract_matches};
1010
use itertools::{Itertools, chain};
1111

1212
use super::block_builder::BlockStructRecomposer;
13+
use super::context::VarRequest;
1314
use crate::VariableId;
1415
use crate::ids::LocationId;
1516

@@ -119,31 +120,30 @@ impl<'db> SemanticLoweringMapping<'db> {
119120
) -> Result<VariableId, MovedVar<'db>> {
120121
match value {
121122
Value::Var(var) => Ok(*var),
123+
Value::MovedVar(moved) => Err(moved.clone()),
122124
Value::Scattered(scattered) => {
123-
let members_res = scattered
125+
let mut moved_var = None;
126+
let members = scattered
124127
.members
125128
.iter_mut()
126-
.map(|(_, value)| Self::assemble_value(ctx, value))
127-
.collect::<Result<_, _>>();
128-
129-
match members_res {
130-
Ok(members) => {
131-
let var = ctx.reconstruct(scattered.concrete_struct_id, members);
132-
*value = Value::Var(var);
133-
Ok(var)
134-
}
135-
Err(MovedVar { ty: _, inference_error, last_use_location }) => {
136-
// Don't use the type of the moved member. Replace it with the type of the
137-
// variable.
138-
let y = TypeLongId::<'db>::Concrete(ConcreteTypeId::Struct(
139-
scattered.concrete_struct_id,
140-
));
141-
let x = y.intern(ctx.ctx.db);
142-
Err(MovedVar { ty: x, inference_error, last_use_location })
143-
}
129+
.map(|(_, value)| match Self::assemble_value(ctx, value) {
130+
Ok(var) => var,
131+
Err(moved) => {
132+
let var = moved.var_id;
133+
moved_var.get_or_insert(moved);
134+
var
135+
}
136+
})
137+
.collect_vec();
138+
let var = ctx.reconstruct(scattered.concrete_struct_id, members);
139+
*value = Value::Var(var);
140+
if let Some(MovedVar { var_id: _, inference_error, last_use_location }) = moved_var
141+
{
142+
Err(MovedVar { var_id: var, inference_error, last_use_location })
143+
} else {
144+
Ok(var)
144145
}
145146
}
146-
Value::MovedVar(moved) => Err(moved.clone()),
147147
}
148148
}
149149

@@ -170,13 +170,14 @@ impl<'db> SemanticLoweringMapping<'db> {
170170
let scattered = Scattered { concrete_struct_id: *concrete_struct_id, members };
171171
*parent_value = Value::Scattered(Box::new(scattered));
172172
}
173-
Value::MovedVar(MovedVar { ty: _, inference_error, last_use_location }) => {
173+
Value::MovedVar(MovedVar { var_id: var, inference_error, last_use_location }) => {
174174
let member_map = ctx.ctx.db.concrete_struct_members(*concrete_struct_id).unwrap();
175+
let location = ctx.ctx.variables[*var].location;
175176
let members = OrderedHashMap::from_iter(member_map.values().map(|member| {
176177
(
177178
member.id,
178179
Value::MovedVar(MovedVar {
179-
ty: member.ty,
180+
var_id: ctx.ctx.new_var(VarRequest { ty: member.ty, location }),
180181
inference_error: inference_error.clone(),
181182
last_use_location: *last_use_location,
182183
}),
@@ -369,7 +370,7 @@ pub fn find_changed_members<'db, 'a>(
369370
#[debug_db(ExprFormatter<'db>)]
370371
pub struct MovedVar<'db> {
371372
/// The type of the variable.
372-
pub ty: semantic::TypeId<'db>,
373+
pub var_id: VariableId,
373374
/// The reason it is not copyable.
374375
pub inference_error: InferenceError<'db>,
375376
/// The location of the last use of the moved variable. This is used to report an error.

0 commit comments

Comments
 (0)