Skip to content

Commit 75196c0

Browse files
DaniPopesDaniPopes
authored
feat(typeck): check lvalue (#641)
More tests, add reason, make it somewhat work.
1 parent 138f831 commit 75196c0

22 files changed

+586
-28
lines changed

‎crates/sema/src/typeck/checker.rs‎

Lines changed: 78 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ use crate::{
66
use alloy_primitives::U256;
77
use solar_ast::{DataLocation, ElementaryType, Span};
88
use solar_data_structures::{Never, map::FxHashMap, pluralize, smallvec::SmallVec};
9-
use solar_interface::diagnostics::DiagCtxt;
9+
use solar_interface::{diagnostics::DiagCtxt, sym};
1010
use std::ops::ControlFlow;
1111

1212
pub(super) fn check(gcx: Gcx<'_>, source: hir::SourceId) {
@@ -21,7 +21,18 @@ struct TypeChecker<'gcx> {
2121

2222
types: FxHashMap<hir::ExprId, Ty<'gcx>>,
2323

24-
lvalue_context: Option<bool>,
24+
lvalue_context: Option<Result<(), NotLvalueReason>>,
25+
}
26+
27+
#[derive(Clone, Copy)]
28+
enum NotLvalueReason {
29+
Constant,
30+
Immutable,
31+
CalldataArray,
32+
CalldataStruct,
33+
FixedBytesIndex,
34+
ArrayLength,
35+
Generic,
2536
}
2637

2738
impl<'gcx> TypeChecker<'gcx> {
@@ -164,7 +175,7 @@ impl<'gcx> TypeChecker<'gcx> {
164175
};
165176

166177
if !is_array_push {
167-
self.not_lvalue();
178+
self.try_set_not_lvalue(NotLvalueReason::Generic);
168179
}
169180

170181
ty
@@ -181,8 +192,8 @@ impl<'gcx> TypeChecker<'gcx> {
181192
}
182193
hir::ExprKind::Ident(res) => {
183194
let res = self.resolve_overloads(res, expr.span);
184-
if !res_is_lvalue(self.gcx, res) {
185-
self.not_lvalue();
195+
if let Some(reason) = res_not_lvalue_reason(self.gcx, res) {
196+
self.try_set_not_lvalue(reason);
186197
}
187198
self.type_of_res(res)
188199
}
@@ -192,7 +203,11 @@ impl<'gcx> TypeChecker<'gcx> {
192203
return ty;
193204
}
194205
if ty.loc() == Some(DataLocation::Calldata) {
195-
self.not_lvalue();
206+
self.try_set_not_lvalue(NotLvalueReason::CalldataArray);
207+
}
208+
if matches!(ty.peel_refs().kind, TyKind::Elementary(ElementaryType::FixedBytes(_)))
209+
{
210+
self.try_set_not_lvalue(NotLvalueReason::FixedBytesIndex);
196211
}
197212
if let Some((index_ty, result_ty)) = self.index_types(ty) {
198213
// Index expression.
@@ -283,18 +298,36 @@ impl<'gcx> TypeChecker<'gcx> {
283298
};
284299

285300
// Validate lvalue.
286-
match expr_ty.kind {
287-
TyKind::Ref(_, d) if d.is_calldata() => self.not_lvalue(),
301+
let not_lvalue_reason = match expr_ty.kind {
302+
_ if matches!(
303+
expr_ty.peel_refs().kind,
304+
TyKind::Array(..) | TyKind::DynArray(_)
305+
) && possible_members.len() == 1
306+
&& possible_members[0].name == sym::length =>
307+
{
308+
Some(NotLvalueReason::ArrayLength)
309+
}
310+
TyKind::Ref(inner, d) if d.is_calldata() => {
311+
let reason = if matches!(inner.kind, TyKind::Struct(_)) {
312+
NotLvalueReason::CalldataStruct
313+
} else {
314+
NotLvalueReason::CalldataArray
315+
};
316+
Some(reason)
317+
}
288318
TyKind::Type(ty)
289319
if matches!(ty.kind, TyKind::Contract(_))
290320
&& possible_members.len() == 1
291-
&& !possible_members[0]
292-
.res
293-
.is_some_and(|res| res_is_lvalue(self.gcx, res)) =>
321+
&& possible_members[0].res.is_some_and(|res| {
322+
res_not_lvalue_reason(self.gcx, res).is_some()
323+
}) =>
294324
{
295-
self.not_lvalue();
325+
Some(NotLvalueReason::Generic)
296326
}
297-
_ => {}
327+
_ => None,
328+
};
329+
if let Some(reason) = not_lvalue_reason {
330+
self.try_set_not_lvalue(reason);
298331
}
299332

300333
ty
@@ -636,26 +669,36 @@ impl<'gcx> TypeChecker<'gcx> {
636669

637670
#[must_use]
638671
fn require_lvalue(&mut self, expr: &'gcx hir::Expr<'gcx>) -> Ty<'gcx> {
639-
let prev = self.lvalue_context.replace(true);
672+
let prev = self.lvalue_context.replace(Ok(()));
640673
let ty = self.check_expr(expr);
641-
let ctx = self.lvalue_context;
642-
debug_assert!(ctx.is_some());
674+
let result = self.lvalue_context.unwrap();
643675
self.lvalue_context = prev;
644-
// TODO: check ctx
645-
if is_syntactic_lvalue(expr) {
676+
677+
if result.is_ok() && is_syntactic_lvalue(expr) {
646678
return ty;
647679
}
648680

649-
// TODO: better error message https://github.com/ethereum/solidity/blob/9d7cc42bc1c12bb43e9dccf8c6c36833fdfcbbca/libsolidity/analysis/TypeChecker.cpp#L4143
650-
651-
self.dcx().err("expected lvalue").span(expr.span).emit();
681+
let msg = match result {
682+
Err(NotLvalueReason::Constant) => "cannot assign to a constant variable",
683+
Err(NotLvalueReason::Immutable) => "cannot assign to an immutable variable",
684+
Err(NotLvalueReason::CalldataArray) => "calldata arrays are read-only",
685+
Err(NotLvalueReason::CalldataStruct) => "calldata structs are read-only",
686+
Err(NotLvalueReason::FixedBytesIndex) => {
687+
"single bytes in fixed bytes arrays cannot be modified"
688+
}
689+
Err(NotLvalueReason::ArrayLength) => {
690+
"member `length` is read-only and cannot be used to resize arrays"
691+
}
692+
Err(NotLvalueReason::Generic) | Ok(()) => "expression has to be an lvalue",
693+
};
694+
self.dcx().err(msg).span(expr.span).emit();
652695

653696
ty
654697
}
655698

656-
fn not_lvalue(&mut self) {
657-
if let Some(v) = &mut self.lvalue_context {
658-
*v = false;
699+
fn try_set_not_lvalue(&mut self, reason: NotLvalueReason) {
700+
if let Some(Ok(())) = self.lvalue_context {
701+
self.lvalue_context = Some(Err(reason));
659702
}
660703
}
661704

@@ -915,11 +958,18 @@ impl<T> FromIterator<T> for WantOne<T> {
915958
}
916959
}
917960

918-
fn res_is_lvalue(gcx: Gcx<'_>, res: hir::Res) -> bool {
961+
fn res_not_lvalue_reason(gcx: Gcx<'_>, res: hir::Res) -> Option<NotLvalueReason> {
919962
match res {
920-
hir::Res::Item(hir::ItemId::Variable(var)) => !gcx.hir.variable(var).is_constant(),
921-
hir::Res::Err(_) => true,
922-
_ => false,
963+
hir::Res::Item(hir::ItemId::Variable(var)) => {
964+
let var = gcx.hir.variable(var);
965+
match var.mutability {
966+
Some(m) if m.is_constant() => Some(NotLvalueReason::Constant),
967+
Some(m) if m.is_immutable() => Some(NotLvalueReason::Immutable),
968+
_ => None,
969+
}
970+
}
971+
hir::Res::Err(_) => None,
972+
_ => Some(NotLvalueReason::Generic),
923973
}
924974
}
925975

‎tests/ui/typeck/lvalue/array_length.sol‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
//@compile-flags: -Ztypeck
2+
// TODO: `mismatched types` errors on integer literals are a current limitation of solar
3+
4+
contract Test {
5+
uint256[] dynamicArray;
6+
uint256[10] fixedArray; //~ ERROR: mismatched types
7+
uint256 state;
8+
9+
function testDynamic() external {
10+
dynamicArray.length = state; //~ ERROR: member `length` is read-only and cannot be used to resize arrays
11+
}
12+
13+
function testFixed() external {
14+
fixedArray.length = state; //~ ERROR: member `length` is read-only and cannot be used to resize arrays
15+
}
16+
17+
function testParam(uint256[] memory arr) internal {
18+
arr.length = state; //~ ERROR: member `length` is read-only and cannot be used to resize arrays
19+
}
20+
}

‎tests/ui/typeck/lvalue/array_length.stderr‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
error: mismatched types
2+
╭▸ ROOT/tests/ui/typeck/lvalue/array_length.sol:LL:CC
3+
│
4+
LL │ uint256[10] fixedArray;
5+
╰╴ ━━ expected `uint256`, found `int_literal[1]`
6+
7+
error: member `length` is read-only and cannot be used to resize arrays
8+
╭▸ ROOT/tests/ui/typeck/lvalue/array_length.sol:LL:CC
9+
│
10+
LL │ dynamicArray.length = state;
11+
╰╴ ━━━━━━━━━━━━━━━━━━━
12+
13+
error: member `length` is read-only and cannot be used to resize arrays
14+
╭▸ ROOT/tests/ui/typeck/lvalue/array_length.sol:LL:CC
15+
│
16+
LL │ fixedArray.length = state;
17+
╰╴ ━━━━━━━━━━━━━━━━━
18+
19+
error: member `length` is read-only and cannot be used to resize arrays
20+
╭▸ ROOT/tests/ui/typeck/lvalue/array_length.sol:LL:CC
21+
│
22+
LL │ arr.length = state;
23+
╰╴ ━━━━━━━━━━
24+
25+
error: aborting due to 4 previous errors
26+

‎tests/ui/typeck/lvalue/calldata_array.sol‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@compile-flags: -Ztypeck
2+
3+
contract Test {
4+
uint256 state;
5+
uint256 idx;
6+
7+
function test(uint256[] calldata arr) external {
8+
arr[idx] = state; //~ ERROR: calldata arrays are read-only
9+
}
10+
11+
function testBytes(bytes calldata data) external {
12+
bytes1 b;
13+
data[idx] = b; //~ ERROR: calldata arrays are read-only
14+
}
15+
16+
function testNested(uint256[][] calldata nested) external {
17+
nested[idx][idx] = state; //~ ERROR: calldata arrays are read-only
18+
}
19+
}

‎tests/ui/typeck/lvalue/calldata_array.stderr‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
error: calldata arrays are read-only
2+
╭▸ ROOT/tests/ui/typeck/lvalue/calldata_array.sol:LL:CC
3+
│
4+
LL │ arr[idx] = state;
5+
╰╴ ━━━━━━━━
6+
7+
error: calldata arrays are read-only
8+
╭▸ ROOT/tests/ui/typeck/lvalue/calldata_array.sol:LL:CC
9+
│
10+
LL │ data[idx] = b;
11+
╰╴ ━━━━━━━━━
12+
13+
error: calldata arrays are read-only
14+
╭▸ ROOT/tests/ui/typeck/lvalue/calldata_array.sol:LL:CC
15+
│
16+
LL │ nested[idx][idx] = state;
17+
╰╴ ━━━━━━━━━━━━━━━━
18+
19+
error: aborting due to 3 previous errors
20+

‎tests/ui/typeck/lvalue/calldata_struct.sol‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
//@compile-flags: -Ztypeck
2+
3+
struct S {
4+
uint256 x;
5+
uint256 y;
6+
}
7+
8+
struct Nested {
9+
S inner;
10+
}
11+
12+
contract Test {
13+
uint256 state;
14+
15+
function test(S calldata s) external {
16+
s.x = state; //~ ERROR: calldata structs are read-only
17+
}
18+
19+
function testNested(Nested calldata n) external {
20+
n.inner.x = state; //~ ERROR: calldata structs are read-only
21+
}
22+
}

‎tests/ui/typeck/lvalue/calldata_struct.stderr‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
error: calldata structs are read-only
2+
╭▸ ROOT/tests/ui/typeck/lvalue/calldata_struct.sol:LL:CC
3+
│
4+
LL │ s.x = state;
5+
╰╴ ━━━
6+
7+
error: calldata structs are read-only
8+
╭▸ ROOT/tests/ui/typeck/lvalue/calldata_struct.sol:LL:CC
9+
│
10+
LL │ n.inner.x = state;
11+
╰╴ ━━━━━━━━━
12+
13+
error: aborting due to 2 previous errors
14+

‎tests/ui/typeck/lvalue/constant.sol‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
//@compile-flags: -Ztypeck
2+
// TODO: `mismatched types` errors on integer literals are a current limitation of solar
3+
4+
contract Test {
5+
uint256 constant CONST = 1; //~ ERROR: mismatched types
6+
7+
function test() external {
8+
CONST = 2; //~ ERROR: cannot assign to a constant variable
9+
//~^ ERROR: mismatched types
10+
}
11+
}
12+
13+
uint256 constant FILE_CONST = 1; //~ ERROR: mismatched types
14+
15+
function fileLevel() {
16+
FILE_CONST = 2; //~ ERROR: cannot assign to a constant variable
17+
//~^ ERROR: mismatched types
18+
}

‎tests/ui/typeck/lvalue/constant.stderr‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
error: mismatched types
2+
╭▸ ROOT/tests/ui/typeck/lvalue/constant.sol:LL:CC
3+
│
4+
LL │ uint256 constant CONST = 1;
5+
╰╴ ━ expected `uint256`, found `int_literal[1]`
6+
7+
error: cannot assign to a constant variable
8+
╭▸ ROOT/tests/ui/typeck/lvalue/constant.sol:LL:CC
9+
│
10+
LL │ CONST = 2;
11+
╰╴ ━━━━━
12+
13+
error: mismatched types
14+
╭▸ ROOT/tests/ui/typeck/lvalue/constant.sol:LL:CC
15+
│
16+
LL │ CONST = 2;
17+
╰╴ ━ expected `uint256`, found `int_literal[1]`
18+
19+
error: mismatched types
20+
╭▸ ROOT/tests/ui/typeck/lvalue/constant.sol:LL:CC
21+
│
22+
LL │ uint256 constant FILE_CONST = 1;
23+
╰╴ ━ expected `uint256`, found `int_literal[1]`
24+
25+
error: cannot assign to a constant variable
26+
╭▸ ROOT/tests/ui/typeck/lvalue/constant.sol:LL:CC
27+
│
28+
LL │ FILE_CONST = 2;
29+
╰╴ ━━━━━━━━━━
30+
31+
error: mismatched types
32+
╭▸ ROOT/tests/ui/typeck/lvalue/constant.sol:LL:CC
33+
│
34+
LL │ FILE_CONST = 2;
35+
╰╴ ━ expected `uint256`, found `int_literal[1]`
36+
37+
error: aborting due to 6 previous errors
38+

‎tests/ui/typeck/lvalue/expressions.sol‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
//@compile-flags: -Ztypeck
2+
3+
contract Test {
4+
uint256 a;
5+
uint256 b;
6+
int256 c;
7+
8+
function testBinaryOp() external {
9+
(a + b) = a; //~ ERROR: expression has to be an lvalue
10+
}
11+
12+
function testTernary() external {
13+
(true ? a : b) = a; //~ ERROR: expression has to be an lvalue
14+
}
15+
16+
function testUnary() external {
17+
(-c) = c; //~ ERROR: expression has to be an lvalue
18+
}
19+
}

0 commit comments

Comments
 (0)