Skip to content
Open
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
25 changes: 18 additions & 7 deletions crates/parse/src/parser/ty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -114,19 +114,30 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
Ok(ty)
}

/// Parses a mapping key type.
fn parse_mapping_key_type(&mut self) -> PResult<'sess, Type<'ast>> {
if self.check_elementary_type() {
self.parse_spanned(Self::parse_elementary_type)
.map(|(span, kind)| Type { span, kind: TypeKind::Elementary(kind) })
} else if self.check_path() {
self.parse_spanned(Self::parse_path)
.map(|(span, path)| Type { span, kind: TypeKind::Custom(path) })
} else {
self.unexpected()
}
}

/// Parses a mapping type.
fn parse_mapping_type(&mut self) -> PResult<'sess, TypeMapping<'ast>> {
self.expect(TokenKind::OpenDelim(Delimiter::Parenthesis))?;

let key = self.parse_type()?;
// TODO: Move to type checking.
if !key.is_elementary() && !key.is_custom() {
let msg =
"only elementary types or used-defined types can be used as key types in mappings";
self.dcx().err(msg).span(key.span).emit();
}
let key = self.parse_mapping_key_type()?;
let key_name = self.parse_ident_opt()?;

// We are about to require a `=>`. Previous checks (e.g. state mutability on elementary
// types) may have added expectations like `payable`/`pure`/`view`. Clear them so the
// diagnostic only mentions `=>` here.
self.expected_tokens.clear();
self.expect(TokenKind::FatArrow)?;

let value = self.parse_type()?;
Expand Down
17 changes: 16 additions & 1 deletion crates/sema/src/typeck/checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -625,6 +625,18 @@ impl<'gcx> TypeChecker<'gcx> {
// TODO: checks from https://github.com/ethereum/solidity/blob/9d7cc42bc1c12bb43e9dccf8c6c36833fdfcbbca/libsolidity/analysis/TypeChecker.cpp#L1219
}

#[must_use]
fn check_mapping_key_type(&mut self, key: &'gcx hir::Type<'gcx>) -> Ty<'gcx> {
let ty = self.gcx.type_of_hir_ty(key);
if !matches!(
ty.kind,
TyKind::Elementary(_) | TyKind::Udvt(_, _) | TyKind::Contract(_) | TyKind::Enum(_)
) {
self.dcx().err("Only elementary types, user defined value types, contract types or enums are allowed as mapping keys.").span(key.span).emit();
}
ty
}

#[must_use]
fn require_lvalue(&mut self, expr: &'gcx hir::Expr<'gcx>) -> Ty<'gcx> {
let prev = self.lvalue_context.replace(true);
Expand Down Expand Up @@ -731,7 +743,6 @@ impl<'gcx> hir::Visit<'gcx> for TypeChecker<'gcx> {
ControlFlow::Continue(())
}

#[expect(clippy::single_match)]
fn visit_ty(&mut self, hir_ty: &'gcx hir::Type<'gcx>) -> ControlFlow<Self::BreakValue> {
match hir_ty.kind {
hir::TypeKind::Array(array) => {
Expand All @@ -740,6 +751,10 @@ impl<'gcx> hir::Visit<'gcx> for TypeChecker<'gcx> {
}
return self.visit_ty(&array.element);
}
hir::TypeKind::Mapping(mapping) => {
let _ = self.check_mapping_key_type(&mapping.key);
self.visit_ty(&mapping.value)?;
}
// TODO: https://github.com/ethereum/solidity/blob/9d7cc42bc1c12bb43e9dccf8c6c36833fdfcbbca/libsolidity/analysis/TypeChecker.cpp#L713
// hir::TypeKind::Function(func) => {
// if func.visibility == hir::Visibility::External {
Expand Down
20 changes: 20 additions & 0 deletions tests/ui/parser/mapping_key_type.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
type U is int;
enum E {
A,
B
}

library L{
enum E1 {
A,
B
}
}

contract C {
mapping(uint => uint) m0;
mapping(E => uint) m1;
mapping(U => uint) m2;
mapping(L.E1 => uint) m3;
mapping(uint[] => uint) m4; //~ ERROR: expected `=>`, found `[`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the PR overall looks good to me, but I'm mulling over this, I understand that this is technically incorrect syntax but part of me wishes this was a more helpful message. no action needed right now as I'm still figuring out if we want it like this or not

Copy link
Contributor Author

@mablr mablr Oct 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see your point.
This line is actually an edge case because an elementary-type is successfully parsed before falling to parse =>:

mapping(uint[] => uint) m4;

but the following:

mapping([] => uint) m4;

would return a more insightful error message:

error: expected one of elementary type name or path, found `[`

}
8 changes: 8 additions & 0 deletions tests/ui/parser/mapping_key_type.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: expected `=>`, found `[`
--> ROOT/tests/ui/parser/mapping_key_type.sol:LL:CC
|
LL | mapping(uint[] => uint) m4;
| ^ expected `=>`

error: aborting due to 1 previous error

25 changes: 25 additions & 0 deletions tests/ui/typeck/mapping_key_type_check.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
//@compile-flags: -Ztypeck
type U is int;
enum E {
A,
B
}

library L{
enum E1 {
A,
B
}
struct S1 {
int x;
}
}

contract C {
mapping(uint => uint) m0;
mapping(string => uint) m1;
mapping(E => uint) m2;
mapping(U => uint) m3;
mapping(L.E1 => uint) m4;
mapping(L.S1 => uint) m5; //~ ERROR: Only elementary types, user defined value types, contract types or enums are allowed as mapping keys.
}
8 changes: 8 additions & 0 deletions tests/ui/typeck/mapping_key_type_check.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
error: Only elementary types, user defined value types, contract types or enums are allowed as mapping keys.
--> ROOT/tests/ui/typeck/mapping_key_type_check.sol:LL:CC
|
LL | mapping(L.S1 => uint) m5;
| ^^^^

error: aborting due to 1 previous error

Loading