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

Basic unsafe support #569

Merged
merged 1 commit into from
Oct 13, 2021
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
1 change: 1 addition & 0 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion crates/analyzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
2 changes: 1 addition & 1 deletion crates/analyzer/src/db/queries/contracts.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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")],
Expand Down
21 changes: 20 additions & 1 deletion crates/analyzer/src/db/queries/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -25,6 +26,17 @@ pub fn function_signature(
let mut scope = ItemScope::new(db, function.module(db));
let contract = function.contract(db);

if_chain! {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Sweet helper :)

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
Expand Down Expand Up @@ -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
Expand Down
12 changes: 9 additions & 3 deletions crates/analyzer/src/namespace/items.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
1 change: 1 addition & 0 deletions crates/analyzer/src/namespace/scopes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -189,6 +189,7 @@ pub enum BlockScopeType {
Function,
IfElse,
Loop,
Unsafe,
}

impl AnalyzerContext for BlockScope<'_, '_> {
Expand Down
34 changes: 30 additions & 4 deletions crates/analyzer/src/traversal/expressions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
};
Expand All @@ -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;
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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,
Expand All @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
17 changes: 17 additions & 0 deletions crates/analyzer/src/traversal/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(()),
Expand Down Expand Up @@ -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 } => {
Expand Down
2 changes: 2 additions & 0 deletions crates/analyzer/tests/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
51 changes: 51 additions & 0 deletions crates/analyzer/tests/snapshots/errors__unsafe_misuse.snap
Original file line number Diff line number Diff line change
@@ -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


20 changes: 20 additions & 0 deletions crates/analyzer/tests/snapshots/errors__unsafe_nesting.snap
Original file line number Diff line number Diff line change
@@ -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


7 changes: 5 additions & 2 deletions crates/lowering/src/mappers/functions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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),
Expand Down Expand Up @@ -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),
Expand Down
3 changes: 2 additions & 1 deletion crates/lowering/src/mappers/module.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
21 changes: 19 additions & 2 deletions crates/parser/src/ast.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>),
Expand All @@ -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>>,
Expand Down Expand Up @@ -216,6 +219,7 @@ pub enum FuncStmt {
Revert {
error: Option<Node<Expr>>,
},
Unsafe(Vec<Node<FuncStmt>>),
}

#[derive(Serialize, Deserialize, Debug, PartialEq, Eq, Hash, Clone)]
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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!(
Expand Down Expand Up @@ -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))
}
}
}
}
Expand Down
Loading