Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: disallow non-constant file-level variables #103

Merged
merged 1 commit into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
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