Skip to content

Commit

Permalink
fix: disallow non-constant file-level variables
Browse files Browse the repository at this point in the history
  • Loading branch information
DaniPopes committed Nov 5, 2024
1 parent ea4b121 commit 7b9a30e
Show file tree
Hide file tree
Showing 10 changed files with 522 additions and 4 deletions.
23 changes: 23 additions & 0 deletions crates/ast/src/token.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,58 +557,69 @@ impl Token {
}

/// Returns `true` if the token is a keyword used in the language.
#[inline]
pub fn is_used_keyword(&self) -> bool {
self.is_ident_where(Ident::is_used_keyword)
}

/// Returns `true` if the token is a keyword reserved for possible future use.
#[inline]
pub fn is_unused_keyword(&self) -> bool {
self.is_ident_where(Ident::is_unused_keyword)
}

/// Returns `true` if the token is a keyword.
#[inline]
pub fn is_reserved_ident(&self, yul: bool) -> bool {
self.is_ident_where(|i| i.is_reserved(yul))
}

/// Returns `true` if the token is an identifier, but not a keyword.
#[inline]
pub fn is_non_reserved_ident(&self, yul: bool) -> bool {
self.is_ident_where(|i| i.is_non_reserved(yul))
}

/// Returns `true` if the token is an elementary type name.
///
/// Note that this does not include `[u]fixedMxN` types.
#[inline]
pub fn is_elementary_type(&self) -> bool {
self.is_ident_where(Ident::is_elementary_type)
}

/// Returns `true` if the token is the identifier `true` or `false`.
#[inline]
pub fn is_bool_lit(&self) -> bool {
self.is_ident_where(|id| id.name.is_bool_lit())
}

/// Returns `true` if the token is a numeric literal.
#[inline]
pub fn is_numeric_lit(&self) -> bool {
matches!(self.kind, TokenKind::Literal(TokenLitKind::Integer | TokenLitKind::Rational, _))
}

/// Returns `true` if the token is the integer literal.
#[inline]
pub fn is_integer_lit(&self) -> bool {
matches!(self.kind, TokenKind::Literal(TokenLitKind::Integer, _))
}

/// Returns `true` if the token is the rational literal.
#[inline]
pub fn is_rational_lit(&self) -> bool {
matches!(self.kind, TokenKind::Literal(TokenLitKind::Rational, _))
}

/// Returns `true` if the token is a string literal.
#[inline]
pub fn is_str_lit(&self) -> bool {
matches!(self.kind, TokenKind::Literal(TokenLitKind::Str, _))
}

/// Returns `true` if the token is an identifier for which `pred` holds.
#[inline]
pub fn is_ident_where(&self, pred: impl FnOnce(Ident) -> bool) -> bool {
self.ident().map(pred).unwrap_or(false)
}
Expand Down Expand Up @@ -643,6 +654,18 @@ impl Token {
self.is_ident_where(Ident::is_location_specifier)
}

/// Returns `true` if the token is a mutability specifier.
#[inline]
pub fn is_mutability_specifier(&self) -> bool {
self.is_ident_where(Ident::is_mutability_specifier)
}

/// Returns `true` if the token is a visibility specifier.
#[inline]
pub fn is_visibility_specifier(&self) -> bool {
self.is_ident_where(Ident::is_visibility_specifier)
}

/// Returns this token's full description: `{self.description()} '{self.kind}'`.
pub fn full_description(&self) -> impl fmt::Display + '_ {
// https://github.com/rust-lang/rust/blob/44bf2a32a52467c45582c3355a893400e620d010/compiler/rustc_parse/src/parser/mod.rs#L378
Expand Down
3 changes: 1 addition & 2 deletions crates/interface/src/diagnostics/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,7 @@ impl DiagnosticId {
}
}

/// Used for creating an error code. The input must be exactly one 'E' character followed by four
/// decimal digits.
/// Used for creating an error code. The input must be exactly 4 decimal digits.
///
/// # Examples
///
Expand Down
24 changes: 24 additions & 0 deletions crates/interface/src/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,18 @@ impl Ident {
pub fn is_location_specifier(self) -> bool {
self.name.is_location_specifier()
}

/// Returns `true` if the identfier is a mutability specifier.
#[inline]
pub fn is_mutability_specifier(self) -> bool {
self.name.is_mutability_specifier()
}

/// Returns `true` if the identfier is a visibility specifier.
#[inline]
pub fn is_visibility_specifier(self) -> bool {
self.name.is_visibility_specifier()
}
}

/// An interned string.
Expand Down Expand Up @@ -308,6 +320,18 @@ impl Symbol {
matches!(self, kw::Calldata | kw::Memory | kw::Storage)
}

/// Returns `true` if the symbol is a mutability specifier.
#[inline]
pub fn is_mutability_specifier(self) -> bool {
matches!(self, kw::Immutable | kw::Constant)
}

/// Returns `true` if the symbol is a visibility specifier.
#[inline]
pub fn is_visibility_specifier(self) -> bool {
matches!(self, kw::Public | kw::Private | kw::Internal | kw::External)
}

/// Returns `true` if the symbol was interned in the compiler's `symbols!` macro.
#[inline]
pub const fn is_preinterned(self) -> bool {
Expand Down
7 changes: 6 additions & 1 deletion crates/parse/src/parser/item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -742,7 +742,8 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
visibility = Some(v);
}
} else if let Some(m) = self.parse_variable_mutability() {
if !flags.contains(VarFlags::from_varmut(m)) {
// `CONSTANT_VAR` is special cased later.
if flags != VarFlags::CONSTANT_VAR && !flags.contains(VarFlags::from_varmut(m)) {
let msg = varmut_error(m, flags.varmuts());
self.dcx().err(msg).span(self.prev_token.span).emit();
} else if mutability.is_some() {
Expand Down Expand Up @@ -809,6 +810,10 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
let msg = "constant variable must be initialized";
self.dcx().err(msg).span(span).emit();
}
if flags == VarFlags::CONSTANT_VAR && mutability != Some(VarMut::Constant) {
let msg = "only constant variables are allowed at file level";
self.dcx().err(msg).span(span).emit();
}

Ok(VariableDefinition {
span,
Expand Down
7 changes: 6 additions & 1 deletion crates/parse/src/parser/stmt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,12 @@ impl<'sess, 'ast> Parser<'sess, 'ast> {
if self.token.is_elementary_type() && next.is_ident_where(|id| id.name == kw::Payable) {
return LookAheadInfo::VariableDeclaration;
}
if next.is_non_reserved_ident(self.in_yul) || next.is_location_specifier() {
if next.is_non_reserved_ident(self.in_yul)
|| next.is_location_specifier()
// These aren't valid but we include them for a better error message.
|| next.is_mutability_specifier()
|| next.is_visibility_specifier()
{
return LookAheadInfo::VariableDeclaration;
}
if matches!(next.kind, TokenKind::OpenDelim(Delimiter::Bracket) | TokenKind::Dot) {
Expand Down
5 changes: 5 additions & 0 deletions tests/ui/typeck/library_mappings.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// contract L {
// function f(mapping(uint=>uint) storage x, mapping(uint=>uint) storage y) internal {
// // x = y;
// }
// }
46 changes: 46 additions & 0 deletions tests/ui/typeck/var_mutability.sol
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
uint a = 0; //~ ERROR: only constant variables are allowed at file level
uint constant b = 0;
uint immutable c = 0; //~ ERROR: only constant variables are allowed at file level

contract C {
uint a2 = 0;
uint constant b2 = 0;
uint immutable c2 = 0;

struct S {
uint a3;
uint constant b3; //~ ERROR: mutability is not allowed here
uint immutable c3; //~ ERROR: mutability is not allowed here
}

error Er(
uint a4,
uint constant b4, //~ ERROR: mutability is not allowed here
uint immutable c4 //~ ERROR: mutability is not allowed here
);

event Ev(
uint a5,
uint constant b5, //~ ERROR: mutability is not allowed here
uint immutable c5 //~ ERROR: mutability is not allowed here
);

function f(
uint a6,
uint constant b6, //~ ERROR: mutability is not allowed here
uint immutable c6 //~ ERROR: mutability is not allowed here
) public returns(
uint a7,
uint constant b7, //~ ERROR: mutability is not allowed here
uint immutable c7 //~ ERROR: mutability is not allowed here
) {
uint a8;
uint constant b8; //~ ERROR: mutability is not allowed here
uint immutable c8; //~ ERROR: mutability is not allowed here
try this.f(0, 0, 0) returns(
uint a9,
uint constant b9, //~ ERROR: mutability is not allowed here
uint immutable c9 //~ ERROR: mutability is not allowed here
) {} catch {}
}
}
114 changes: 114 additions & 0 deletions tests/ui/typeck/var_mutability.stderr
Original file line number Diff line number Diff line change
@@ -0,0 +1,114 @@
error: only constant variables are allowed at file level
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint a = 0;
| ^^^^^^^^^^^
|

error: only constant variables are allowed at file level
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c = 0;
| ^^^^^^^^^^^^^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b3;
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c3;
| ^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b4,
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c4
| ^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b5,
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c5
| ^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b6,
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c6
| ^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b7,
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c7
| ^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b8;
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c8;
| ^^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint constant b9,
| ^^^^^^^^
|

error: mutability is not allowed here
--> ROOT/tests/ui/typeck/var_mutability.sol:LL:CC
|
LL | uint immutable c9
| ^^^^^^^^^
|

error: aborting due to 16 previous errors

Loading

0 comments on commit 7b9a30e

Please sign in to comment.