Skip to content

Commit 960affc

Browse files
author
Mohammad Fawaz
committed
fix(typeck): reject non-path receivers for Vector/Mapping storage ops
`Vector::{len,get,set,push,pop,swap_remove,clear}` and `Mapping::{get,get_or_use,contains,set,remove}` assumed an `Expression::Path` receiver downstream: storage_lowering for vectors, codegen for mappings. Type-checking only verified the receiver's type. - Read ops (`len`/`get`/`get_or_use`/`contains`) had no path check at all, so a ternary of two storage vectors ICEd in storage_lowering (#29428) and a ternary of two mappings produced invalid bytecode that failed late at disassembly. - Write ops already had an `is_local_path` check, but emitted the "external vectors/mappings" diagnostic for a local ternary receiver, which is misleading. Add an `Expression::Path` pre-check to all eleven arms and emit a new `ETYC0372191` for non-path receivers. The existing "external vectors/mappings" diagnostic now only fires when the receiver is an actual external path, which is what its wording describes. Fixes #29428.
1 parent eade0dc commit 960affc

8 files changed

Lines changed: 367 additions & 4 deletions

File tree

crates/passes/src/errors/type_checker.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -942,6 +942,21 @@ pub(crate) fn multi_identifier_definition_requires_tuple(type_: impl Display, sp
942942
.with_help("Use a tuple expression, e.g. `let (a, b) = (x, y);`.")
943943
}
944944

945+
pub(crate) fn storage_op_requires_path_receiver(
946+
module: impl Display,
947+
operation: impl Display,
948+
kind: impl Display,
949+
span: Span,
950+
) -> Formatted {
951+
Formatted::error(
952+
CODE_PREFIX,
953+
CODE_MASK + 191,
954+
format!("The receiver of `{module}::{operation}` must be a {kind}."),
955+
span,
956+
)
957+
.with_help(format!("Call `{operation}` directly on a declared {kind}, not on a temporary expression."))
958+
}
959+
945960
// TypeCheckerWarning builder functions
946961

947962
pub(crate) fn caller_as_record_owner(record_name: impl Display, span: Span) -> Formatted {

crates/passes/src/type_checking/visitor.rs

Lines changed: 151 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -966,6 +966,19 @@ impl TypeCheckingVisitor<'_> {
966966
// Check that the operation is invoked in a `finalize` or `async` block.
967967
self.check_access_allowed("Mapping::get", true, function_span);
968968

969+
// The receiver must be a path. Codegen lowers `Mapping::get` to an Aleo
970+
// `get` instruction whose first operand is a mapping name, so non-path
971+
// receivers (e.g. a ternary over mappings) cannot be lowered.
972+
if !matches!(map_expr, Expression::Path(_)) {
973+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
974+
"Mapping",
975+
"get",
976+
"mapping",
977+
map_expr.span(),
978+
));
979+
return Type::Err;
980+
}
981+
969982
*value.clone()
970983
}
971984
Intrinsic::MappingSet => {
@@ -979,6 +992,18 @@ impl TypeCheckingVisitor<'_> {
979992
// Check that the operation is invoked in a `finalize` or `async` block.
980993
self.check_access_allowed("Mapping::set", true, function_span);
981994

995+
// Receiver must be a path. Codegen emits an Aleo `set` whose first
996+
// operand is a mapping name.
997+
if !matches!(map_expr, Expression::Path(_)) {
998+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
999+
"Mapping",
1000+
"set",
1001+
"mapping",
1002+
map_expr.span(),
1003+
));
1004+
return Type::Err;
1005+
}
1006+
9821007
// Argument 0 must be a local path (cannot modify external mappings).
9831008
if !is_local_path(map_expr) {
9841009
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
@@ -1005,6 +1030,18 @@ impl TypeCheckingVisitor<'_> {
10051030
return Type::Err;
10061031
};
10071032

1033+
// The receiver must be a path. Codegen lowers `Mapping::get_or_use` to an
1034+
// Aleo `get.or_use` instruction whose first operand is a mapping name.
1035+
if !matches!(map_expr, Expression::Path(_)) {
1036+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1037+
"Mapping",
1038+
"get_or_use",
1039+
"mapping",
1040+
map_expr.span(),
1041+
));
1042+
return Type::Err;
1043+
}
1044+
10081045
value.deref().clone()
10091046
}
10101047
Intrinsic::MappingRemove => {
@@ -1021,6 +1058,18 @@ impl TypeCheckingVisitor<'_> {
10211058
return Type::Err;
10221059
};
10231060

1061+
// Receiver must be a path. Codegen emits an Aleo `remove` whose first
1062+
// operand is a mapping name.
1063+
if !matches!(map_expr, Expression::Path(_)) {
1064+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1065+
"Mapping",
1066+
"remove",
1067+
"mapping",
1068+
map_expr.span(),
1069+
));
1070+
return Type::Err;
1071+
}
1072+
10241073
// Argument 0 must be a local path (cannot modify external mappings).
10251074
if !is_local_path(map_expr) {
10261075
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
@@ -1047,6 +1096,18 @@ impl TypeCheckingVisitor<'_> {
10471096
return Type::Err;
10481097
};
10491098

1099+
// The receiver must be a path. Codegen lowers `Mapping::contains` to an
1100+
// Aleo `contains` instruction whose first operand is a mapping name.
1101+
if !matches!(map_expr, Expression::Path(_)) {
1102+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1103+
"Mapping",
1104+
"contains",
1105+
"mapping",
1106+
map_expr.span(),
1107+
));
1108+
return Type::Err;
1109+
}
1110+
10501111
Type::Boolean
10511112
}
10521113
Intrinsic::OptionalUnwrap => {
@@ -1082,6 +1143,19 @@ impl TypeCheckingVisitor<'_> {
10821143
// Check that the operation is invoked in a `finalize` or `async` block.
10831144
self.check_access_allowed("Vector::get", true, function_span);
10841145

1146+
// The receiver must be a path. storage_lowering looks up the backing
1147+
// mappings by name, so non-path receivers (e.g. a ternary over storage
1148+
// vectors) cannot be lowered.
1149+
if !matches!(vec_expr, Expression::Path(_)) {
1150+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1151+
"Vector",
1152+
"get",
1153+
"storage vector",
1154+
vec_expr.span(),
1155+
));
1156+
return Type::Err;
1157+
}
1158+
10851159
Type::Optional(OptionalType { inner: Box::new(*element_type.clone()) })
10861160
}
10871161
Intrinsic::VectorSet => {
@@ -1095,6 +1169,18 @@ impl TypeCheckingVisitor<'_> {
10951169
// Check that the operation is invoked in a `finalize` or `async` block.
10961170
self.check_access_allowed("Vector::set", true, function_span);
10971171

1172+
// Receiver must be a path (storage_lowering needs to identify the backing
1173+
// mappings by name).
1174+
if !matches!(vec_expr, Expression::Path(_)) {
1175+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1176+
"Vector",
1177+
"set",
1178+
"storage vector",
1179+
vec_expr.span(),
1180+
));
1181+
return Type::Err;
1182+
}
1183+
10981184
// Argument 0 must be a local path (cannot modify external vectors).
10991185
if !is_local_path(vec_expr) {
11001186
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
@@ -1123,6 +1209,18 @@ impl TypeCheckingVisitor<'_> {
11231209
// Check that the operation is invoked in a `finalize` or `async` block.
11241210
self.check_access_allowed("Vector::push", true, function_span);
11251211

1212+
// Receiver must be a path (storage_lowering needs to identify the backing
1213+
// mappings by name).
1214+
if !matches!(vec_expr, Expression::Path(_)) {
1215+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1216+
"Vector",
1217+
"push",
1218+
"storage vector",
1219+
vec_expr.span(),
1220+
));
1221+
return Type::Err;
1222+
}
1223+
11261224
// Argument 0 must be a local path (cannot modify external vectors).
11271225
if !is_local_path(vec_expr) {
11281226
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
@@ -1141,12 +1239,25 @@ impl TypeCheckingVisitor<'_> {
11411239
// Check that the operation is invoked in a `finalize` or `async` block.
11421240
self.check_access_allowed("Vector::len", true, function_span);
11431241

1144-
if vec_ty.is_vector() {
1145-
Type::Integer(IntegerType::U32)
1146-
} else {
1242+
if !vec_ty.is_vector() {
11471243
self.assert_vector_type(vec_ty, vec_expr.span());
1148-
Type::Err
1244+
return Type::Err;
1245+
}
1246+
1247+
// The receiver must be a path. storage_lowering looks up the backing
1248+
// mappings by name, so non-path receivers (e.g. a ternary over storage
1249+
// vectors) cannot be lowered.
1250+
if !matches!(vec_expr, Expression::Path(_)) {
1251+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1252+
"Vector",
1253+
"len",
1254+
"storage vector",
1255+
vec_expr.span(),
1256+
));
1257+
return Type::Err;
11491258
}
1259+
1260+
Type::Integer(IntegerType::U32)
11501261
}
11511262
Intrinsic::VectorPop => {
11521263
let (vec_ty, vec_expr) = &arguments[0];
@@ -1159,6 +1270,18 @@ impl TypeCheckingVisitor<'_> {
11591270
return Type::Err;
11601271
};
11611272

1273+
// Receiver must be a path (storage_lowering needs to identify the backing
1274+
// mappings by name).
1275+
if !matches!(vec_expr, Expression::Path(_)) {
1276+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1277+
"Vector",
1278+
"pop",
1279+
"storage vector",
1280+
vec_expr.span(),
1281+
));
1282+
return Type::Err;
1283+
}
1284+
11621285
// Argument 0 must be a local path (cannot modify external vectors).
11631286
if !is_local_path(vec_expr) {
11641287
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
@@ -1182,6 +1305,18 @@ impl TypeCheckingVisitor<'_> {
11821305
return Type::Err;
11831306
};
11841307

1308+
// Receiver must be a path (storage_lowering needs to identify the backing
1309+
// mappings by name).
1310+
if !matches!(vec_expr, Expression::Path(_)) {
1311+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1312+
"Vector",
1313+
"swap_remove",
1314+
"storage vector",
1315+
vec_expr.span(),
1316+
));
1317+
return Type::Err;
1318+
}
1319+
11851320
// Argument 0 must be a local path (cannot modify external vectors).
11861321
if !is_local_path(vec_expr) {
11871322
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
@@ -1202,6 +1337,18 @@ impl TypeCheckingVisitor<'_> {
12021337
return Type::Err;
12031338
}
12041339

1340+
// Receiver must be a path (storage_lowering needs to identify the backing
1341+
// mappings by name).
1342+
if !matches!(vec_expr, Expression::Path(_)) {
1343+
self.state.handler.emit_err(crate::errors::type_checker::storage_op_requires_path_receiver(
1344+
"Vector",
1345+
"clear",
1346+
"storage vector",
1347+
vec_expr.span(),
1348+
));
1349+
return Type::Err;
1350+
}
1351+
12051352
// Argument 0 must be a local path (cannot modify external vectors).
12061353
if !is_local_path(vec_expr) {
12071354
self.state.handler.emit_err(crate::errors::type_checker::cannot_modify_external_container(
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
[ETYC0372191] Error: The receiver of `Mapping::get` must be a mapping.
2+
╭─[ compiler-test:12:27 ]
3+
4+
12 │ let v: u32 = (c ? m1 : m2).get(0u32); // ❌ non-path receiver
5+
6+
│ Help: Call `get` directly on a declared mapping, not on a temporary expression.
7+
────╯
8+
[ETYC0372191] Error: The receiver of `Mapping::contains` must be a mapping.
9+
╭─[ compiler-test:19:28 ]
10+
11+
19 │ let b: bool = (c ? m1 : m2).contains(0u32); // ❌ non-path receiver
12+
13+
│ Help: Call `contains` directly on a declared mapping, not on a temporary expression.
14+
────╯
15+
[ETYC0372191] Error: The receiver of `Mapping::get_or_use` must be a mapping.
16+
╭─[ compiler-test:26:27 ]
17+
18+
26 │ let v: u32 = (c ? m1 : m2).get_or_use(0u32, 0u32); // ❌ non-path receiver
19+
20+
│ Help: Call `get_or_use` directly on a declared mapping, not on a temporary expression.
21+
────╯
Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,21 @@
1+
[ETYC0372191] Error: The receiver of `Vector::len` must be a storage vector.
2+
╭─[ compiler-test:15:27 ]
3+
4+
15 │ let n: u32 = (c ? h1 : h2).len(); // ❌ non-path receiver
5+
6+
│ Help: Call `len` directly on a declared storage vector, not on a temporary expression.
7+
────╯
8+
[ETYC0372191] Error: The receiver of `Vector::get` must be a storage vector.
9+
╭─[ compiler-test:22:28 ]
10+
11+
22 │ let v: u64? = (c ? h1 : h2).get(0u32); // ❌ non-path receiver
12+
13+
│ Help: Call `get` directly on a declared storage vector, not on a temporary expression.
14+
────╯
15+
[ETYC0372191] Error: The receiver of `Vector::len` must be a storage vector.
16+
╭─[ compiler-test:29:39 ]
17+
18+
29 │ let n: u32 = Vector::len((c ? h1 : h2)); // ❌ non-path receiver
19+
20+
│ Help: Call `len` directly on a declared storage vector, not on a temporary expression.
21+
────╯
Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
[ETYC0372191] Error: The receiver of `Vector::set` must be a storage vector.
2+
╭─[ compiler-test:15:25 ]
3+
4+
15 │ return final { (c ? h1 : h2).set(0u32, 1u64); }; // ❌ non-path receiver
5+
6+
│ Help: Call `set` directly on a declared storage vector, not on a temporary expression.
7+
────╯
8+
[ETYC0372191] Error: The receiver of `Vector::push` must be a storage vector.
9+
╭─[ compiler-test:19:25 ]
10+
11+
19 │ return final { (c ? h1 : h2).push(1u64); }; // ❌ non-path receiver
12+
13+
│ Help: Call `push` directly on a declared storage vector, not on a temporary expression.
14+
────╯
15+
[ETYC0372191] Error: The receiver of `Vector::pop` must be a storage vector.
16+
╭─[ compiler-test:24:28 ]
17+
18+
24 │ let x: u64? = (c ? h1 : h2).pop(); // ❌ non-path receiver
19+
20+
│ Help: Call `pop` directly on a declared storage vector, not on a temporary expression.
21+
────╯
22+
[ETYC0372191] Error: The receiver of `Vector::swap_remove` must be a storage vector.
23+
╭─[ compiler-test:31:27 ]
24+
25+
31 │ let x: u64 = (c ? h1 : h2).swap_remove(0u32); // ❌ non-path receiver
26+
27+
│ Help: Call `swap_remove` directly on a declared storage vector, not on a temporary expression.
28+
────╯
29+
[ETYC0372191] Error: The receiver of `Vector::clear` must be a storage vector.
30+
╭─[ compiler-test:37:25 ]
31+
32+
37 │ return final { (c ? h1 : h2).clear(); }; // ❌ non-path receiver
33+
34+
│ Help: Call `clear` directly on a declared storage vector, not on a temporary expression.
35+
────╯
36+
[ETYC0372191] Error: The receiver of `Mapping::set` must be a mapping.
37+
╭─[ compiler-test:41:25 ]
38+
39+
41 │ return final { (c ? m1 : m2).set(0u32, 1u32); }; // ❌ non-path receiver
40+
41+
│ Help: Call `set` directly on a declared mapping, not on a temporary expression.
42+
────╯
43+
[ETYC0372191] Error: The receiver of `Mapping::remove` must be a mapping.
44+
╭─[ compiler-test:45:25 ]
45+
46+
45 │ return final { (c ? m1 : m2).remove(0u32); }; // ❌ non-path receiver
47+
48+
│ Help: Call `remove` directly on a declared mapping, not on a temporary expression.
49+
────╯
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
// Companion to #29428: ensure non-path receivers for `Mapping::get` /
2+
// `Mapping::contains` / `Mapping::get_or_use` are also caught cleanly.
3+
program bugpoc_map.aleo {
4+
@noupgrade
5+
constructor() {}
6+
7+
mapping m1: u32 => u32;
8+
mapping m2: u32 => u32;
9+
10+
fn method_get(c: bool) -> Final {
11+
return final {
12+
let v: u32 = (c ? m1 : m2).get(0u32); // ❌ non-path receiver
13+
assert(v >= 0u32);
14+
};
15+
}
16+
17+
fn method_contains(c: bool) -> Final {
18+
return final {
19+
let b: bool = (c ? m1 : m2).contains(0u32); // ❌ non-path receiver
20+
assert(b == b);
21+
};
22+
}
23+
24+
fn method_get_or_use(c: bool) -> Final {
25+
return final {
26+
let v: u32 = (c ? m1 : m2).get_or_use(0u32, 0u32); // ❌ non-path receiver
27+
assert(v >= 0u32);
28+
};
29+
}
30+
}

0 commit comments

Comments
 (0)