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

require initialization of structs and arrays at decl site #747

Merged
merged 1 commit into from
Jul 5, 2022
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 crates/analyzer/src/traversal/const_expr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ pub(crate) fn eval_expr(
| ast::Expr::Attribute { .. }
| ast::Expr::Call { .. }
| ast::Expr::List { .. }
| ast::Expr::Repeat { .. }
| ast::Expr::Tuple { .. }
| ast::Expr::Unit => Err(not_const_error(context, expr.span)),
}
Expand Down
18 changes: 18 additions & 0 deletions crates/analyzer/src/traversal/declarations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,24 @@ pub fn var_decl(scope: &mut BlockScope, stmt: &Node<fe::FuncStmt>) -> Result<(),
value_attributes.typ,
);
}
} else if matches!(declared_type.typ(scope.db()), Type::Array(_)) {
scope.error(
"uninitialized variable",
target.span,
"array types must be initialized at declaration site",
);
} else if matches!(declared_type.typ(scope.db()), Type::Struct(_)) {
scope.error(
"uninitialized variable",
target.span,
"struct types must be initialized at declaration site",
);
} else if matches!(declared_type.typ(scope.db()), Type::Tuple(_)) {
scope.error(
"uninitialized variable",
target.span,
"tuple types must be initialized at declaration site",
);
}

add_var(scope, target, declared_type)?;
Expand Down
72 changes: 68 additions & 4 deletions crates/analyzer/src/traversal/expressions.rs
Original file line number Diff line number Diff line change
@@ -1,21 +1,25 @@
use crate::builtins::{ContractTypeMethod, GlobalFunction, Intrinsic, ValueMethod};
use crate::context::{AnalyzerContext, CallType, ExpressionAttributes, Location, NamedThing};
use crate::context::{
AnalyzerContext, CallType, Constant, ExpressionAttributes, Location, NamedThing,
};
use crate::display::Displayable;
use crate::errors::{FatalError, IndexingError};
use crate::namespace::items::{Class, FunctionId, Item, StructId, TypeDef};
use crate::namespace::scopes::BlockScopeType;
use crate::namespace::types::{Array, Base, FeString, Integer, Tuple, Type, TypeDowncast, TypeId};
use crate::operations;
use crate::traversal::call_args::{validate_arg_count, validate_named_args};
use crate::traversal::const_expr::eval_expr;
use crate::traversal::types::apply_generic_type_args;
use crate::traversal::utils::add_bin_operations_errors;

use fe_common::diagnostics::Label;
use fe_common::{numeric, Span};
use fe_parser::ast as fe;
use fe_parser::ast::UnaryOperator;
use fe_parser::ast::GenericArg;
use fe_parser::node::Node;
use num_bigint::BigInt;
use num_traits::ToPrimitive;
use smol_str::SmolStr;
use std::ops::RangeInclusive;
use std::rc::Rc;
Expand Down Expand Up @@ -45,9 +49,9 @@ pub fn expr(
args,
} => expr_call(context, func, generic_args, args),
fe::Expr::List { elts } => expr_list(context, elts, expected_type),
fe::Expr::Repeat { .. } => expr_repeat(context, exp, expected_type),
fe::Expr::Tuple { .. } => expr_tuple(context, exp, expected_type),
fe::Expr::Str(_) => expr_str(context, exp, expected_type),

fe::Expr::Bool(_) => Ok(ExpressionAttributes::new(
TypeId::bool(context.db()),
Location::Value,
Expand Down Expand Up @@ -134,6 +138,66 @@ pub fn expr_list(
})
}

pub fn expr_repeat(
context: &mut dyn AnalyzerContext,
expr: &Node<fe::Expr>,
expected_type: Option<TypeId>,
) -> Result<ExpressionAttributes, FatalError> {
let (value, len) = if let fe::Expr::Repeat { value, len } = &expr.kind {
(value, len)
} else {
unreachable!()
};

let value = assignable_expr(context, value, None)?;

let size = match &len.kind {
GenericArg::Int(size) => size.kind,
GenericArg::TypeDesc(_) => {
return Err(FatalError::new(context.fancy_error(
"expected a constant u256 value",
vec![Label::primary(len.span, "Array length")],
vec!["Note: Array length must be a constant u256".to_string()],
)));
}
GenericArg::ConstExpr(expr) => {
assignable_expr(context, expr, None)?;
if let Constant::Int(len) = eval_expr(context, expr)? {
len.to_usize().unwrap()
} else {
return Err(FatalError::new(context.fancy_error(
"expected a constant u256 value",
vec![Label::primary(len.span, "Array length")],
vec!["Note: Array length must be a constant u256".to_string()],
)));
}
}
};

let array_typ = context.db().intern_type(Type::Array(Array {
size,
inner: value.typ,
}));

if let Some(expected_typ) = expected_type {
if expected_typ.typ(context.db()) != array_typ.typ(context.db()) {
return Err(FatalError::new(context.type_error(
"type mismatch",
expr.span,
expected_typ,
array_typ,
)));
}
}

Ok(ExpressionAttributes {
typ: array_typ,
location: Location::Memory,
move_location: None,
const_value: None,
})
}

/// Gather context information for expressions and check for type errors.
///
/// Also ensures that the expression is on the stack.
Expand Down Expand Up @@ -781,7 +845,7 @@ fn expr_unary_operation(
Location::Value,
))
}
UnaryOperator::Invert => {
fe::UnaryOperator::Invert => {
if !operand_attributes.typ.is_integer(context.db()) {
emit_err(context, "a numeric type")
}
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 @@ -341,3 +341,5 @@ test_file! { ctx_builtins_param_incorrect_type }
test_file! { ctx_undefined_create }
test_file! { ctx_undefined_create2 }
test_file! { ctx_undefined_event }
test_file! { uninit_values }
test_file! { invalid_repeat_length }
16 changes: 13 additions & 3 deletions crates/analyzer/tests/snapshots/analysis__aug_assign.snap
Original file line number Diff line number Diff line change
Expand Up @@ -264,7 +264,7 @@ note:
┌─ aug_assign.fe:65:5
65 │ ╭ pub fn add_from_mem(a: u256, b: u256) -> u256 {
66 │ │ let my_array: Array<u256, 10>
66 │ │ let my_array: Array<u256, 10> = [0; 10]
Copy link
Collaborator

Choose a reason for hiding this comment

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

Adding this repeat expression feels a bit like a crutch to me. It doesn't seem overly useful for the user since it only allows a single value to be repeated. Did you consider to just initialize the array with default values on behalf of the user. Maybe that gets complicated if the array is over a complex type such as Array<Customer, 10>. However, in the future we could have a Default trait and if a user wants to initialize an array of Array<Customer, 10> then it will automatically use the values provided by Customer::default() or else error if Customer doesn't implement the Default trait. I'm just thinking out loud here...I'm not really against introducing the repeat expression but if in the future we find a way to get rid of it again or make it more useful I wouldn't be sad to see it go 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ok, I just learned that one can use the same approach to initialize fixed size arrays in Rust. So, I guess it's fine. I still think that in the future it could be nice syntactical sugar if one could just write let x: Array<Customer, 10> and it would de-sugar into something like let x: Array<Customer, 10> = [Customer::default(), 10]. And if T does not implement the Default trait it would cause a compile error.

67 │ │ my_array[7] = a
68 │ │ my_array[7] += b
69 │ │ return my_array[7]
Expand All @@ -274,12 +274,22 @@ note:
note:
┌─ aug_assign.fe:66:13
66let my_array: Array<u256, 10>
66let my_array: Array<u256, 10> = [0; 10]
^^^^^^^^ Array<u256, 10>

note:
┌─ aug_assign.fe:67:9
┌─ aug_assign.fe:66:42
66let my_array: Array<u256, 10> = [0; 10]
^ ^^ u256: Value
│ │
u256: Value

note:
┌─ aug_assign.fe:66:41
66let my_array: Array<u256, 10> = [0; 10]
^^^^^^^ Array<u256, 10>: Memory
67my_array[7] = a
^^^^^^^^ ^ u256: Value
│ │
Expand Down
Loading