this is not a keyword (#1317)

this should not be a keyword, and variables can be named this although this will hide the usual this which gives the current contract. Signed-off-by: Sean Young <[email protected]>
hyperledger · May 19, 2023 · 1a47714 · 1a47714
1 parent 5a871ef
commit 1a47714
Show file tree

Hide file tree

Showing 14 changed files with 45 additions and 42 deletions.
diff --git a/solang-parser/src/helpers/fmt.rs b/solang-parser/src/helpers/fmt.rs
@@ -428,8 +428,6 @@ impl pt::ContractTy {
 impl Display for pt::Expression {
     fn fmt(&self, f: &mut Formatter<'_>) -> Result {
         match self {
-            Self::This(_) => f.write_str("this"),
-
             Self::New(_, expr) => {
                 f.write_str("new ")?;
                 expr.fmt(f)
@@ -678,7 +676,6 @@ impl pt::Expression {
             | Variable(..)
             | List(..)
             | ArrayLiteral(..)
-            | This(..)
             | Parenthesis(..) => return None,
         };
         Some(operator)
@@ -2032,8 +2029,6 @@ mod tests {
             }
 
             pt::Expression: {
-                pt::Expression::This(loc!()) => "this",
-
                 pt::Expression::New(loc!(), Box::new(expr_ty!(uint256))) => "new uint256",
                 pt::Expression::Delete(loc!(), Box::new(expr_ty!(uint256))) => "delete uint256",
 

diff --git a/solang-parser/src/helpers/loc.rs b/solang-parser/src/helpers/loc.rs
@@ -355,7 +355,6 @@ impl_for_enums! {
         | Self::ArrayLiteral(l, ..)
         | Self::List(l, ..)
         | Self::Type(l, ..)
-        | Self::This(l, ..)
         | Self::AddressLiteral(l, ..) => l,
     }
 

diff --git a/solang-parser/src/lexer.rs b/solang-parser/src/lexer.rs
@@ -161,7 +161,6 @@ pub enum Token<'input> {
     Receive,
     Fallback,
 
-    This,
     As,
     Is,
     Abstract,
@@ -299,7 +298,6 @@ impl<'input> fmt::Display for Token<'input> {
             Token::Catch => write!(f, "catch"),
             Token::Receive => write!(f, "receive"),
             Token::Fallback => write!(f, "fallback"),
-            Token::This => write!(f, "this"),
             Token::As => write!(f, "as"),
             Token::Is => write!(f, "is"),
             Token::Abstract => write!(f, "abstract"),
@@ -541,7 +539,6 @@ static KEYWORDS: phf::Map<&'static str, Token> = phf_map! {
     "catch" => Token::Catch,
     "receive" => Token::Receive,
     "fallback" => Token::Fallback,
-    "this" => Token::This,
     "as" => Token::As,
     "is" => Token::Is,
     "abstract" => Token::Abstract,

diff --git a/solang-parser/src/pt.rs b/solang-parser/src/pt.rs
@@ -1075,8 +1075,6 @@ pub enum Expression {
     List(Loc, ParameterList),
     /// `\[ <1>.* \]`
     ArrayLiteral(Loc, Vec<Expression>),
-    /// The `this` keyword.
-    This(Loc),
 }
 
 /// See `Expression::components`.
@@ -1147,8 +1145,7 @@ macro_rules! expr_components {
             | AddressLiteral(..)
             | Variable(..)
             | List(..)
-            | ArrayLiteral(..)
-            | This(..) => (None, None),
+            | ArrayLiteral(..) => (None, None),
         }
     };
 }
@@ -1226,7 +1223,6 @@ impl Expression {
                 | HexLiteral(..)
                 | AddressLiteral(..)
                 | Variable(..)
-                | This(..)
         )
     }
 

diff --git a/solang-parser/src/solidity.lalrpop b/solang-parser/src/solidity.lalrpop
@@ -489,7 +489,6 @@ NoFunctionTyPrecedence0: Expression = {
 
         Expression::List(Loc::File(file_no, l, r), a)
     },
-    <@L> "this" <@R> => Expression::This(Loc::File(file_no, <>)),
     LiteralExpression
 }
 
@@ -1226,7 +1225,6 @@ extern {
         "catch" => Token::Catch,
         "receive" => Token::Receive,
         "fallback" => Token::Fallback,
-        "this" => Token::This,
         "as" => Token::As,
         "is" => Token::Is,
         "abstract" => Token::Abstract,

diff --git a/solang-parser/src/tests.rs b/solang-parser/src/tests.rs
@@ -54,8 +54,8 @@ contract 9c {
                 Diagnostic { loc: File(0, 460, 461), level: Error, ty: ParserError, message: "unrecognised token '!', expected \"!=\", \"%\", \"%=\", \"&\", \"&&\", \"&=\", \")\", \"*\", \"**\", \"*=\", \"+\", \"++\", \"+=\", \",\", \"-\", \"--\", \"-=\", \".\", \"/\", \"/=\", \":\", \";\", \"<\", \"<<\", \"<<=\", \"<=\", \"=\", \"==\", \"=>\", \">\", \">=\", \">>\", \">>=\", \"?\", \"[\", \"]\", \"^\", \"^=\", \"calldata\", \"case\", \"constant\", \"default\", \"external\", \"immutable\", \"indexed\", \"internal\", \"leave\", \"memory\", \"override\", \"payable\", \"private\", \"public\", \"pure\", \"return\", \"returns\", \"revert\", \"storage\", \"switch\", \"view\", \"virtual\", \"{\", \"|\", \"|=\", \"||\", \"}\", identifier".to_string(), notes: vec![] },
                 Diagnostic { loc: File(0, 482, 483), level: Error, ty: ParserError, message: "unrecognised token '3', expected \"!=\", \"%\", \"%=\", \"&\", \"&&\", \"&=\", \"(\", \")\", \"*\", \"**\", \"*=\", \"+\", \"++\", \"+=\", \",\", \"-\", \"--\", \"-=\", \".\", \"/\", \"/=\", \":\", \";\", \"<\", \"<<\", \"<<=\", \"<=\", \"=\", \"==\", \"=>\", \">\", \">=\", \">>\", \">>=\", \"?\", \"[\", \"]\", \"^\", \"^=\", \"calldata\", \"case\", \"constant\", \"default\", \"external\", \"immutable\", \"indexed\", \"internal\", \"leave\", \"memory\", \"override\", \"private\", \"public\", \"revert\", \"storage\", \"switch\", \"{\", \"|\", \"|=\", \"||\", \"}\", identifier".to_string(), notes: vec![] },
                 Diagnostic { loc: File(0, 518, 522), level: Error, ty: ParserError, message: "unrecognised token 'uint256', expected \"!=\", \"%\", \"%=\", \"&\", \"&&\", \"&=\", \")\", \"*\", \"**\", \"*=\", \"+\", \"++\", \"+=\", \",\", \"-\", \"--\", \"-=\", \".\", \"/\", \"/=\", \":\", \";\", \"<\", \"<<\", \"<<=\", \"<=\", \"=\", \"==\", \">\", \">=\", \">>\", \">>=\", \"?\", \"[\", \"]\", \"^\", \"^=\", \"case\", \"default\", \"leave\", \"switch\", \"|\", \"|=\", \"||\", \"}\", identifier".to_string(), notes: vec![] },
-                Diagnostic { loc: File(0, 555, 556), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"!\", \"(\", \"+\", \"++\", \"-\", \"--\", \"[\", \"address\", \"assembly\", \"bool\", \"break\", \"byte\", \"bytes\", \"case\", \"continue\", \"default\", \"delete\", \"do\", \"emit\", \"false\", \"for\", \"function\", \"if\", \"leave\", \"mapping\", \"new\", \"payable\", \"return\", \"revert\", \"string\", \"switch\", \"this\", \"true\", \"try\", \"type\", \"unchecked\", \"while\", \"{\", \"~\", Bytes, Int, Uint, address, hexnumber, hexstring, identifier, number, rational, string".to_string(), notes: vec![] },
-                Diagnostic { loc: File(0, 557, 558), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"(\", \";\", \"@\", \"[\", \"abstract\", \"address\", \"bool\", \"byte\", \"bytes\", \"case\", \"contract\", \"default\", \"enum\", \"event\", \"false\", \"function\", \"import\", \"interface\", \"leave\", \"library\", \"mapping\", \"payable\", \"pragma\", \"string\", \"struct\", \"switch\", \"this\", \"true\", \"type\", \"using\", Bytes, Int, Uint, address, hexnumber, hexstring, identifier, number, rational, string".to_string(), notes: vec![] }
+                Diagnostic { loc: File(0, 555, 556), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"!\", \"(\", \"+\", \"++\", \"-\", \"--\", \"[\", \"address\", \"assembly\", \"bool\", \"break\", \"byte\", \"bytes\", \"case\", \"continue\", \"default\", \"delete\", \"do\", \"emit\", \"false\", \"for\", \"function\", \"if\", \"leave\", \"mapping\", \"new\", \"payable\", \"return\", \"revert\", \"string\", \"switch\", \"true\", \"try\", \"type\", \"unchecked\", \"while\", \"{\", \"~\", Bytes, Int, Uint, address, hexnumber, hexstring, identifier, number, rational, string".to_string(), notes: vec![] },
+                Diagnostic { loc: File(0, 557, 558), level: Error, ty: ParserError, message: "unrecognised token '}', expected \"(\", \";\", \"@\", \"[\", \"abstract\", \"address\", \"bool\", \"byte\", \"bytes\", \"case\", \"contract\", \"default\", \"enum\", \"event\", \"false\", \"function\", \"import\", \"interface\", \"leave\", \"library\", \"mapping\", \"payable\", \"pragma\", \"string\", \"struct\", \"switch\", \"true\", \"type\", \"using\", Bytes, Int, Uint, address, hexnumber, hexstring, identifier, number, rational, string".to_string(), notes: vec![] }
             ]
         )
     }

diff --git a/src/sema/builtin.rs b/src/sema/builtin.rs
@@ -890,7 +890,7 @@ pub fn builtin_namespace(namespace: &str) -> bool {
 
 /// Is name reserved for builtins
 pub fn is_reserved(fname: &str) -> bool {
-    if fname == "type" || fname == "super" {
+    if fname == "type" || fname == "super" || fname == "this" {
         return true;
     }
 

diff --git a/src/sema/expression/resolve_expression.rs b/src/sema/expression/resolve_expression.rs
@@ -23,7 +23,7 @@ use crate::sema::{
     unused_variable::{check_function_call, check_var_usage_expression, used_variable},
     Recurse,
     {
-        ast::{Builtin, Expression, Namespace, RetrieveType, Type},
+        ast::{Expression, Namespace, RetrieveType, Type},
         diagnostics::Diagnostics,
         eval::check_term_for_constant_overflow,
     },
@@ -720,20 +720,5 @@ pub fn expression(
             ));
             Err(())
         }
-        pt::Expression::This(loc) => match context.contract_no {
-            Some(contract_no) => Ok(Expression::Builtin {
-                loc: *loc,
-                tys: vec![Type::Contract(contract_no)],
-                kind: Builtin::GetAddress,
-                args: Vec::new(),
-            }),
-            None => {
-                diagnostics.push(Diagnostic::error(
-                    *loc,
-                    "this not allowed outside contract".to_owned(),
-                ));
-                Err(())
-            }
-        },
     }
 }
diff --git a/src/sema/expression/variable.rs b/src/sema/expression/variable.rs
@@ -1,6 +1,6 @@
 // SPDX-License-Identifier: Apache-2.0
 
-use crate::sema::ast::{Expression, Namespace, Symbol, Type};
+use crate::sema::ast::{Builtin, Expression, Namespace, Symbol, Type};
 use crate::sema::builtin;
 use crate::sema::diagnostics::Diagnostics;
 use crate::sema::expression::function_call::available_functions;
@@ -151,6 +151,21 @@ pub(super) fn variable(
                     ));
             Err(())
         }
+        None if id.name == "this" => match context.contract_no {
+            Some(contract_no) => Ok(Expression::Builtin {
+                loc: id.loc,
+                tys: vec![Type::Contract(contract_no)],
+                kind: Builtin::GetAddress,
+                args: Vec::new(),
+            }),
+            None => {
+                diagnostics.push(Diagnostic::error(
+                    id.loc,
+                    "this not allowed outside contract".to_owned(),
+                ));
+                Err(())
+            }
+        },
         sym => {
             diagnostics.push(Namespace::wrong_symbol(sym, id));
             Err(())

diff --git a/src/sema/namespace.rs b/src/sema/namespace.rs
@@ -77,12 +77,10 @@ impl Namespace {
         symbol: Symbol,
     ) -> bool {
         if builtin::is_reserved(&id.name) {
-            self.diagnostics.push(Diagnostic::error(
+            self.diagnostics.push(Diagnostic::warning(
                 id.loc,
                 format!("'{}' shadows name of a builtin", id.name),
             ));
-
-            return false;
         }
 
         if let Some(Symbol::Function(v)) =

diff --git a/tests/contract_testcases/evm/this.sol b/tests/contract_testcases/evm/this.sol
@@ -0,0 +1,19 @@
+function test1() returns (address) {
+	return address(this);
+}
+
+function test2(int this) returns (int) {
+	return this * 3;
+}
+
+contract that {
+	// We can shadow this with another variable
+	function foo(int this, int super) public pure returns (int) {
+		return this + super;
+	}
+}
+// ---- Expect: diagnostics ----
+// error: 2:17-21: this not allowed outside contract
+// warning: 5:20-24: 'this' shadows name of a builtin
+// warning: 11:19-23: 'this' shadows name of a builtin
+// warning: 11:29-34: 'super' shadows name of a builtin
diff --git a/tests/contract_testcases/substrate/inheritance/test_super.sol b/tests/contract_testcases/substrate/inheritance/test_super.sol
@@ -1,3 +1,4 @@
 contract super {}
 // ---- Expect: diagnostics ----
-// error: 1:10-15: 'super' shadows name of a builtin
+// error: 1:1-18: contracts without public storage or functions are not allowed on Substrate. Consider declaring this contract abstract: 'abstract contract super'
+// warning: 1:10-15: 'super' shadows name of a builtin
diff --git a/tests/evm.rs b/tests/evm.rs
@@ -249,7 +249,7 @@ fn ethereum_solidity_tests() {
         })
         .sum();
 
-    assert_eq!(errors, 1079);
+    assert_eq!(errors, 1078);
 }
 
 fn set_file_contents(source: &str, path: &Path) -> (FileResolver, Vec<String>) {

diff --git a/vscode/src/test/suite/extension.test.ts b/vscode/src/test/suite/extension.test.ts
@@ -29,7 +29,7 @@ suite('Extension Test Suite', function () {
     await testdiagnos(diagnosdoc2, [
       {
         message:
-          `unrecognised token '}', expected "!", "(", "+", "++", "-", "--", "[", "address", "bool", "byte", "bytes", "case", "default", "delete", "false", "function", "leave", "mapping", "new", "payable", "revert", "string", "switch", "this", "true", "type", "~", Bytes, Int, Uint, address, hexnumber, hexstring, identifier, number, rational, string`,
+          `unrecognised token '}', expected "!", "(", "+", "++", "-", "--", "[", "address", "bool", "byte", "bytes", "case", "default", "delete", "false", "function", "leave", "mapping", "new", "payable", "revert", "string", "switch", "true", "type", "~", Bytes, Int, Uint, address, hexnumber, hexstring, identifier, number, rational, string`,
         range: toRange(13, 1, 13, 2),
         severity: vscode.DiagnosticSeverity.Error,
         source: 'solidity',