Merge pull request #569 from sbillig/unsafe

Basic `unsafe` support

crates/analyzer/Cargo.toml

@@ -18,7 +18,7 @@ semver = "1.0.0"
 salsa = "0.16.1"
 parking_lot_core = { version = "=0.8.0" } # used by salsa; version pinned for wasm compatibility
 indexmap = "1.6.2"
-
+if_chain = "1.0.1"
 
 [dev-dependencies]
 insta = "1.7.1"

crates/analyzer/src/db/queries/contracts.rs

@@ -162,7 +162,7 @@ pub fn contract_init_function(
     if let Some((id, span)) = first_def {
         // `__init__` must be `pub`.
         // Return type is checked in `queries::functions::function_signature`.
-        if !id.data(db).ast.kind.is_pub {
+        if !id.is_public(db) {
             diagnostics.push(errors::fancy_error(
                 "`__init__` function is not public",
                 vec![Label::primary(span, "`__init__` function must be public")],

crates/analyzer/src/db/queries/functions.rs

@@ -9,6 +9,7 @@ use crate::traversal::types::type_desc;
 use fe_common::diagnostics::Label;
 use fe_parser::ast;
 use fe_parser::node::Node;
+use if_chain::if_chain;
 use std::collections::HashMap;
 use std::convert::TryInto;
 use std::rc::Rc;
@@ -25,6 +26,17 @@ pub fn function_signature(
     let mut scope = ItemScope::new(db, function.module(db));
     let contract = function.contract(db);
 
+    if_chain! {
+        if contract.is_some();
+        if let Some(pub_span) = function.pub_span(db);
+        if let Some(unsafe_span) = function.unsafe_span(db);
+        then {
+            scope.error("public contract functions can't be unsafe",
+                        pub_span + unsafe_span,
+                        "a contract function can be either `pub` or `unsafe`, but not both");
+        }
+    }
+
     let mut self_decl = SelfDecl::None;
     let mut names = HashMap::new();
     let params = def
@@ -161,7 +173,14 @@ pub fn function_body(db: &dyn AnalyzerDb, function: FunctionId) -> Analysis<Rc<F
         }
     }
 
-    let mut block_scope = BlockScope::new(&scope, BlockScopeType::Function);
+    let mut block_scope = BlockScope::new(
+        &scope,
+        if function.unsafe_span(db).is_some() {
+            BlockScopeType::Unsafe
+        } else {
+            BlockScopeType::Function
+        },
+    );
 
     // If `traverse_statements` fails, we can be confident that a diagnostic
     // has been emitted, either while analyzing this fn body or while analyzing

crates/analyzer/src/namespace/items.rs

@@ -514,12 +514,18 @@ impl FunctionId {
     pub fn module(&self, db: &dyn AnalyzerDb) -> ModuleId {
         self.data(db).module
     }
-    pub fn is_public(&self, db: &dyn AnalyzerDb) -> bool {
-        self.data(db).ast.kind.is_pub
-    }
     pub fn is_pure(&self, db: &dyn AnalyzerDb) -> bool {
         self.signature(db).self_decl == SelfDecl::None
     }
+    pub fn is_public(&self, db: &dyn AnalyzerDb) -> bool {
+        self.pub_span(db).is_some()
+    }
+    pub fn pub_span(&self, db: &dyn AnalyzerDb) -> Option<Span> {
+        self.data(db).ast.kind.pub_
+    }
+    pub fn unsafe_span(&self, db: &dyn AnalyzerDb) -> Option<Span> {
+        self.data(db).ast.kind.unsafe_
+    }
     pub fn signature(&self, db: &dyn AnalyzerDb) -> Rc<types::FunctionSignature> {
         db.function_signature(*self).value
     }

crates/analyzer/src/namespace/scopes.rs

@@ -189,6 +189,7 @@ pub enum BlockScopeType {
     Function,
     IfElse,
     Loop,
+    Unsafe,
 }
 
 impl AnalyzerContext for BlockScope<'_, '_> {

crates/analyzer/src/traversal/expressions.rs

@@ -5,7 +5,7 @@ use crate::builtins::{
 use crate::context::{AnalyzerContext, CallType, ExpressionAttributes, Location, NamedThing};
 use crate::errors::{FatalError, IndexingError, NotFixedSize};
 use crate::namespace::items::{ContractId, FunctionId, Item};
-use crate::namespace::scopes::BlockScope;
+use crate::namespace::scopes::{BlockScope, BlockScopeType};
 use crate::namespace::types::{
     Array, Base, Contract, FeString, Integer, SelfDecl, Struct, Tuple, Type, TypeDowncast, U256,
 };
@@ -19,6 +19,7 @@ use fe_common::Span;
 use fe_parser::ast as fe;
 use fe_parser::ast::UnaryOperator;
 use fe_parser::node::Node;
+use if_chain::if_chain;
 use num_bigint::BigInt;
 use std::convert::TryInto;
 use std::ops::RangeInclusive;
@@ -732,7 +733,7 @@ fn expr_call(
                 func_name,
                 self_span,
             } => expr_call_self_attribute(scope, &func_name, func.span, self_span, args),
-            CallType::Pure(func_id) => expr_call_pure(scope, func_id, args),
+            CallType::Pure(func_id) => expr_call_pure(scope, func.span, func_id, args),
             CallType::ValueAttribute => expr_call_value_attribute(scope, func, args),
             CallType::TypeAttribute { typ, func_name } => {
                 expr_call_type_attribute(scope, typ, &func_name, func.span, args)
@@ -963,6 +964,27 @@ fn resolve_self(scope: &mut BlockScope, use_span: Span) -> Result<ContractId, Fa
     Ok(contract)
 }
 
+fn check_for_unsafe_call_outside_unsafe(
+    scope: &mut BlockScope,
+    fn_name: &str,
+    call_name_span: Span,
+    function: FunctionId,
+) {
+    if_chain! {
+        if !scope.inherits_type(BlockScopeType::Unsafe);
+        if let Some(unsafe_span) = function.unsafe_span(scope.db());
+        then {
+            let def_name_span = function.name_span(scope.db());
+            scope.fancy_error(&format!("unsafe function `{}` can only be called in an unsafe function or block",
+                                       fn_name),
+                              vec![Label::primary(call_name_span, "call to unsafe function"),
+                                   Label::secondary(unsafe_span + def_name_span, format!("`{}` is defined here as unsafe", fn_name))],
+                              vec!["Hint: put this call in an `unsafe` block if you're confident that it's safe to use here".into()],
+            );
+        }
+    }
+}
+
 fn expr_call_self_attribute(
     scope: &mut BlockScope,
     func_name: &str,
@@ -974,6 +996,8 @@ fn expr_call_self_attribute(
     let contract = resolve_self(scope, self_span)?;
 
     if let Some(func) = contract.self_function(scope.db(), func_name) {
+        check_for_unsafe_call_outside_unsafe(scope, func_name, name_span, func);
+
         let sig = func.signature(scope.root.db);
         validate_named_args(
             scope,
@@ -1013,18 +1037,20 @@ fn expr_call_self_attribute(
 
 fn expr_call_pure(
     scope: &mut BlockScope,
+    call_name_span: Span,
     function: FunctionId,
     args: &Node<Vec<Node<fe::CallArg>>>,
 ) -> Result<ExpressionAttributes, FatalError> {
     assert!(function.is_pure(scope.db()));
 
     let fn_name = function.name(scope.db());
-    let name_span = function.name_span(scope.db());
+    check_for_unsafe_call_outside_unsafe(scope, &fn_name, call_name_span, function);
+
     let sig = function.signature(scope.db());
     validate_named_args(
         scope,
         &fn_name,
-        name_span,
+        function.name_span(scope.db()),
         args,
         &sig.params,
         LabelPolicy::AllowAnyUnlabeled,

crates/analyzer/src/traversal/functions.rs

@@ -29,6 +29,7 @@ fn func_stmt(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(), Fa
         For { .. } => for_loop(scope, stmt),
         While { .. } => while_loop(scope, stmt),
         If { .. } => if_statement(scope, stmt),
+        Unsafe { .. } => unsafe_block(scope, stmt),
         Assert { .. } => assert(scope, stmt),
         Expr { value } => expressions::expr(scope, value, None).map(|_| ()),
         Pass => Ok(()),
@@ -110,6 +111,22 @@ fn if_statement(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(),
     }
 }
 
+fn unsafe_block(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(), FatalError> {
+    match &stmt.kind {
+        fe::FuncStmt::Unsafe(body) => {
+            if scope.inherits_type(BlockScopeType::Unsafe) {
+                scope.error(
+                    "unnecessary `unsafe` block",
+                    stmt.span,
+                    "this `unsafe` block is nested inside another `unsafe` context",
+                );
+            }
+            traverse_statements(&mut scope.new_child(BlockScopeType::Unsafe), body)
+        }
+        _ => unreachable!(),
+    }
+}
+
 fn while_loop(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(), FatalError> {
     match &stmt.kind {
         fe::FuncStmt::While { test, body } => {

crates/analyzer/tests/errors.rs

@@ -257,3 +257,5 @@ test_file! { call_to_pure_fn_on_self }
 test_file! { missing_self }
 test_file! { self_not_first }
 test_file! { self_in_standalone_fn }
+test_file! { unsafe_misuse }
+test_file! { unsafe_nesting }

crates/analyzer/tests/snapshots/errors__unsafe_misuse.snap

@@ -0,0 +1,51 @@
+---
+source: crates/analyzer/tests/errors.rs
+expression: "error_string(&path, &src)"
+
+---
+error: unsafe function `mod_priv` can only be called in an unsafe function or block
+   ┌─ compile_errors/unsafe_misuse.fe:10:3
+   │
+ 3 │ unsafe fn mod_priv(): # OK
+   │ ------------------ `mod_priv` is defined here as unsafe
+   ·
+10 │   mod_priv() # BAD
+   │   ^^^^^^^^ call to unsafe function
+   │
+   = Hint: put this call in an `unsafe` block if you're confident that it's safe to use here
+
+error: public contract functions can't be unsafe
+   ┌─ compile_errors/unsafe_misuse.fe:18:3
+   │
+18 │   pub unsafe fn pub_self(self): # BAD
+   │   ^^^^^^^^^^ a contract function can be either `pub` or `unsafe`, but not both
+
+error: public contract functions can't be unsafe
+   ┌─ compile_errors/unsafe_misuse.fe:20:3
+   │
+20 │   pub unsafe fn pub_noself(): # BAD
+   │   ^^^^^^^^^^ a contract function can be either `pub` or `unsafe`, but not both
+
+error: unsafe function `priv_self` can only be called in an unsafe function or block
+   ┌─ compile_errors/unsafe_misuse.fe:38:5
+   │
+31 │   unsafe fn priv_self(self): # OK
+   │   ------------------- `priv_self` is defined here as unsafe
+   ·
+38 │     self.priv_self() # BAD
+   │     ^^^^^^^^^^^^^^ call to unsafe function
+   │
+   = Hint: put this call in an `unsafe` block if you're confident that it's safe to use here
+
+error: unsafe function `priv_nonself` can only be called in an unsafe function or block
+   ┌─ compile_errors/unsafe_misuse.fe:39:5
+   │
+24 │   unsafe fn priv_nonself(): # OK
+   │   ---------------------- `priv_nonself` is defined here as unsafe
+   ·
+39 │     priv_nonself()   # BAD
+   │     ^^^^^^^^^^^^ call to unsafe function
+   │
+   = Hint: put this call in an `unsafe` block if you're confident that it's safe to use here
+
+

crates/analyzer/tests/snapshots/errors__unsafe_nesting.snap

@@ -0,0 +1,20 @@
+---
+source: crates/analyzer/tests/errors.rs
+expression: "error_string(&path, &src)"
+
+---
+error: unnecessary `unsafe` block
+  ┌─ compile_errors/unsafe_nesting.fe:2:3
+  │  
+2 │ ╭   unsafe:
+3 │ │     g()
+  │ ╰───────^ this `unsafe` block is nested inside another `unsafe` context
+
+error: unnecessary `unsafe` block
+  ┌─ compile_errors/unsafe_nesting.fe:8:5
+  │  
+8 │ ╭     unsafe:
+9 │ │       h()
+  │ ╰─────────^ this `unsafe` block is nested inside another `unsafe` context
+
+

crates/lowering/src/mappers/functions.rs

@@ -13,7 +13,8 @@ use fe_parser::node::Node;
 pub fn func_def(context: &mut ModuleContext, function: FunctionId) -> Node<fe::Function> {
     let node = &function.data(context.db).ast;
     let fe::Function {
-        is_pub,
+        pub_,
+        unsafe_,
         name,
         args,
         return_type: return_type_node,
@@ -89,7 +90,8 @@ pub fn func_def(context: &mut ModuleContext, function: FunctionId) -> Node<fe::F
         .unwrap_or_else(|| fe::TypeDesc::Unit.into_node());
 
     let lowered_function = fe::Function {
-        is_pub: *is_pub,
+        pub_: *pub_,
+        unsafe_: *unsafe_,
         name: name.clone(),
         args,
         return_type: Some(lowered_return_type),
@@ -149,6 +151,7 @@ fn func_stmt(context: &mut FnContext, stmt: Node<fe::FuncStmt>) -> Vec<Node<fe::
             body: multiple_stmts(context, body),
             or_else: multiple_stmts(context, or_else),
         }],
+        fe::FuncStmt::Unsafe(body) => vec![fe::FuncStmt::Unsafe(multiple_stmts(context, body))],
         fe::FuncStmt::Assert { test, msg } => vec![fe::FuncStmt::Assert {
             test: expressions::expr(context, test),
             msg: expressions::optional_expr(context, msg),

crates/lowering/src/mappers/module.rs

@@ -183,7 +183,8 @@ fn list_expr_to_fn_def(array: &Array) -> ast::Function {
 
     // Put it all together in one AST node that holds the entire function definition
     ast::Function {
-        is_pub: false,
+        pub_: None,
+        unsafe_: None,
         name: names::list_expr_generator_fn_name(array).into_node(),
         args,
         return_type,

crates/parser/src/ast.rs

@@ -125,6 +125,7 @@ pub struct Field {
     pub value: Option<Node<Expr>>,
 }
 
+#[allow(clippy::large_enum_variant)]
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
 pub enum ContractStmt {
     Event(Node<Event>),
@@ -139,7 +140,9 @@ pub struct Event {
 
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
 pub struct Function {
-    pub is_pub: bool,
+    // qualifier order: `pub unsafe fn`
+    pub pub_: Option<Span>,
+    pub unsafe_: Option<Span>,
     pub name: Node<String>,
     pub args: Vec<Node<FunctionArg>>,
     pub return_type: Option<Node<TypeDesc>>,
@@ -216,6 +219,7 @@ pub enum FuncStmt {
     Revert {
         error: Option<Node<Expr>>,
     },
+    Unsafe(Vec<Node<FuncStmt>>),
 }
 
 #[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
@@ -344,6 +348,15 @@ impl Node<Field> {
     }
 }
 
+impl Function {
+    pub fn is_pub(&self) -> bool {
+        self.pub_.is_some()
+    }
+    pub fn is_unsafe(&self) -> bool {
+        self.unsafe_.is_some()
+    }
+}
+
 impl Node<Function> {
     pub fn name(&self) -> &str {
         &self.kind.name.kind
@@ -561,7 +574,7 @@ impl fmt::Display for Event {
 
 impl fmt::Display for Function {
     fn fmt(&self, f: &mut Formatter<'_>) -> fmt::Result {
-        if self.is_pub {
+        if self.is_pub() {
             write!(f, "pub ")?;
         }
         write!(
@@ -667,6 +680,10 @@ impl fmt::Display for FuncStmt {
                     write!(f, "revert")
                 }
             }
+            FuncStmt::Unsafe(body) => {
+                writeln!(f, "unsafe:")?;
+                writeln!(indented(f), "{}", node_line_joined(body))
+            }
         }
     }
 }