Skip to content

Commit

Permalink
Dereferencing as reassignment target (#5923)
Browse files Browse the repository at this point in the history
## Description

This PR implements dereferencing in reassignment targets, as defined in
[references](https://github.com/FuelLabs/sway-rfcs/blob/ironcev/amend-references/files/0010-references.sw).
The overall effort related to references is tracked in
#5063.

Up to now, it was only possible to read the values references refer to.
This PR allows values to be written over `&mut` references. In other
words, any `*<expression that results in &mut>`is now an l-values and
can be used in left-hand sides (LHS) of assignments. In most of the
cases, the `<expression>` will very likely be just a reference variable,
but any expression that results in `&mut` is allowed and supported.
E.g.;

```Sway
*mut_ref_to_u64 = 42;

*if condition { &mut x } else { &mut y } = 42;

*max_mut(&mut x, &mut y) = 42;
```

Additionally, the PR:
- fixes #5736 by properly replacing decls in reassignment LHSs.
- fixes #5737 by properly substituting types in reassignment LHSs.
- fixes #5920 by properly connecting expressions in array indices to the
DCA graph.
- fixes #5922 by type-cheking the array indices in reassignment LHSs and
forcing them to be `u64`.
- improves misplaced and misleading error messages when assigning to
constants and other items (see demo below).
- improves error message when assigning to immutable variables by
pointing to variable declaration.
- improves error message when expression cannot be assigned to by
pointing to the problematic part of the expression. Since Sway is more
restrictive here then Rust, the restrictions, without being explained,
could cause confusion. That's why special attention was given to point
to the exact issue and explain what is actually supported in Sway (see
demo below).
- reduces number of expected warnings in references E2E tests that were
previously set high because of DCA issues that are fixed in the
meantime.

The PR also brings additional analysis and checks to IR optimizations.
Among other things, it explicitly points out the cases in which a
potential usage of a symbol cannot be deterministically confirmed or
denied. In this PR properly reacting for such cases is done in some
optimizations. Rolling it out fully will be done in a follow up PR
#5924. More advanced escape analysis will be done later on, as a part of
allocating values on the heap in case of referencing.

This PR implements only dereferencing in LHS. Support for referenced
elements in the element based access (without dereferencing) will be
done in a separate step as an ongoing work on implementing #5063.

Closes #5736, #5737, #5920, #5922.

## Demo

Before:
![Assignment to constant -
Before](https://github.com/FuelLabs/sway/assets/4142833/e6b29d31-8e29-4ffa-acfe-a844ac50887b)

![Assignment to decl -
Before](https://github.com/FuelLabs/sway/assets/4142833/ef5f025e-3dcc-4cb0-b52f-d735e2ee451e)

After:
![Assignment to constant -
After](https://github.com/FuelLabs/sway/assets/4142833/f3dfdbd9-2123-4a96-98a7-b3c7a3f3e7f9)

![Assignment to decl -
After](https://github.com/FuelLabs/sway/assets/4142833/ae765d61-41b2-478c-96c0-d476604deec6)

Expression cannot be assigned to:

![Expression cannot be assigned
to](https://github.com/FuelLabs/sway/assets/4142833/384d0bb7-34a1-446a-acd5-6c8f66dc4ed5)

## Checklist

- [x] I have linked to any relevant issues.
- [x] I have commented my code, particularly in hard-to-understand
areas.
- [x] I have updated the documentation where relevant (API docs, the
reference, and the Sway book).
- [ ] If my change requires substantial documentation changes, I have
[requested support from the DevRel
team](https://github.com/FuelLabs/devrel-requests/issues/new/choose)
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [ ] I have added (or requested a maintainer to add) the necessary
`Breaking*` or `New Feature` labels where relevant.
- [x] I have done my best to ensure that my PR adheres to [the Fuel Labs
Code Review
Standards](https://github.com/FuelLabs/rfcs/blob/master/text/code-standards/external-contributors.md).
- [x] I have requested a review from the relevant team or maintainers.
  • Loading branch information
ironcev authored Apr 30, 2024
1 parent 850a2c4 commit daee1c2
Show file tree
Hide file tree
Showing 110 changed files with 4,329 additions and 665 deletions.
7 changes: 7 additions & 0 deletions Cargo.lock

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

12 changes: 12 additions & 0 deletions docs/book/src/basics/variables.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ foo = 6;

Now, `foo` is mutable, and the reassignment to the number `6` is valid. That is, we are allowed to _mutate_ the variable `foo` to change its value.

When assigning to a mutable variable, the right-hand side of the assignment is evaluated before the left-hand side. In the below example, the mutable variable `i` will first be increased and the resulting value of `1` will be stored to `array[1]`, thus resulting in `array` being changed to `[0, 1, 0]`.

```sway
let mut array = [0, 0, 0];
let mut i = 0;
array[i] = {
i += 1;
i
};
```

## Type Annotations

<!-- This section should explain type annotations -->
Expand Down
12 changes: 12 additions & 0 deletions docs/reference/src/code/language/variables/src/lib.sw
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,18 @@ fn mutable() {
// ANCHOR_END: mutable
}

fn mutable_evaluation_order() {
// ANCHOR: mutable_evaluation_order
let mut array = [0, 0, 0];
let mut i = 0;

array[i] = {
i += 1;
i
};
// ANCHOR_END: mutable_evaluation_order
}

fn immutable() {
// ANCHOR: immutable
let foo = 5;
Expand Down
6 changes: 6 additions & 0 deletions docs/reference/src/documentation/language/variables/let.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,3 +19,9 @@ We can declare a variable that can have its value changed through the use of the
```sway
{{#include ../../../code/language/variables/src/lib.sw:mutable}}
```

When assigning to a mutable variable, the right-hand side of the assignment is evaluated before the left-hand side. In the below example, the mutable variable `i` will first be increased and the resulting value of `1` will be stored to `array[1]`, thus resulting in `array` being changed to `[0, 1, 0]`.

```sway
{{#include ../../../code/language/variables/src/lib.sw:mutable_evaluation_order}}
```
46 changes: 39 additions & 7 deletions sway-ast/src/assignable.rs
Original file line number Diff line number Diff line change
@@ -1,19 +1,42 @@
use crate::priv_prelude::*;

/// Left-hand side of an assignment.
#[derive(Clone, Debug, Serialize)]
pub enum Assignable {
/// A single variable or a path to a part of an aggregate.
/// E.g.:
/// - `my_variable`
/// - `array[0].field.x.1`
/// Note that within the path, we cannot have dereferencing
/// (except, of course, in expressions inside of array index operator).
/// This is guaranteed by the grammar.
/// E.g., an expression like this is not allowed by the grammar:
/// `my_struct.*expr`
ElementAccess(ElementAccess),
/// Dereferencing of an arbitrary reference expression.
/// E.g.:
/// - *my_ref
/// - **if x > 0 { &mut &mut a } else { &mut &mut b }
Deref {
star_token: StarToken,
expr: Box<Expr>,
},
}

#[derive(Clone, Debug, Serialize)]
pub enum ElementAccess {
Var(Ident),
Index {
target: Box<Assignable>,
target: Box<ElementAccess>,
arg: SquareBrackets<Box<Expr>>,
},
FieldProjection {
target: Box<Assignable>,
target: Box<ElementAccess>,
dot_token: DotToken,
name: Ident,
},
TupleFieldProjection {
target: Box<Assignable>,
target: Box<ElementAccess>,
dot_token: DotToken,
field: BigUint,
field_span: Span,
Expand All @@ -23,12 +46,21 @@ pub enum Assignable {
impl Spanned for Assignable {
fn span(&self) -> Span {
match self {
Assignable::Var(name) => name.span(),
Assignable::Index { target, arg } => Span::join(target.span(), arg.span()),
Assignable::FieldProjection { target, name, .. } => {
Assignable::ElementAccess(element_access) => element_access.span(),
Assignable::Deref { star_token, expr } => Span::join(star_token.span(), expr.span()),
}
}
}

impl Spanned for ElementAccess {
fn span(&self) -> Span {
match self {
ElementAccess::Var(name) => name.span(),
ElementAccess::Index { target, arg } => Span::join(target.span(), arg.span()),
ElementAccess::FieldProjection { target, name, .. } => {
Span::join(target.span(), name.span())
}
Assignable::TupleFieldProjection {
ElementAccess::TupleFieldProjection {
target, field_span, ..
} => Span::join(target.span(), field_span.clone()),
}
Expand Down
100 changes: 77 additions & 23 deletions sway-ast/src/expr/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use sway_error::handler::ErrorEmitted;

use crate::{priv_prelude::*, PathExprSegment};
use crate::{assignable::ElementAccess, priv_prelude::*, PathExprSegment};

pub mod asm;
pub mod op_code;
Expand Down Expand Up @@ -446,56 +446,59 @@ impl Spanned for ExprStructField {
}

impl Expr {
/// Returns the resulting [Assignable] if the `self` is a
/// valid [Assignable], or an error containing the [Expr]
/// which causes the `self` to be an invalid [Assignable].
///
/// In case of an error, the returned [Expr] can be `self`
/// or any subexpression of `self` that is not allowed
/// in assignment targets.
pub fn try_into_assignable(self) -> Result<Assignable, Expr> {
if let Expr::Deref { star_token, expr } = self {
Ok(Assignable::Deref { star_token, expr })
} else {
Ok(Assignable::ElementAccess(self.try_into_element_access()?))
}
}

fn try_into_element_access(self) -> Result<ElementAccess, Expr> {
match self {
Expr::Path(path_expr) => match path_expr.try_into_ident() {
Ok(name) => Ok(Assignable::Var(name)),
Ok(name) => Ok(ElementAccess::Var(name)),
Err(path_expr) => Err(Expr::Path(path_expr)),
},
Expr::Index { target, arg } => match target.try_into_assignable() {
Ok(target) => Ok(Assignable::Index {
target: Box::new(target),
arg,
}),
Err(target) => Err(Expr::Index {
Expr::Index { target, arg } => match target.try_into_element_access() {
Ok(target) => Ok(ElementAccess::Index {
target: Box::new(target),
arg,
}),
error => error,
},
Expr::FieldProjection {
target,
dot_token,
name,
} => match target.try_into_assignable() {
Ok(target) => Ok(Assignable::FieldProjection {
target: Box::new(target),
dot_token,
name,
}),
Err(target) => Err(Expr::FieldProjection {
} => match target.try_into_element_access() {
Ok(target) => Ok(ElementAccess::FieldProjection {
target: Box::new(target),
dot_token,
name,
}),
error => error,
},
Expr::TupleFieldProjection {
target,
dot_token,
field,
field_span,
} => match target.try_into_assignable() {
Ok(target) => Ok(Assignable::TupleFieldProjection {
target: Box::new(target),
dot_token,
field,
field_span,
}),
Err(target) => Err(Expr::TupleFieldProjection {
} => match target.try_into_element_access() {
Ok(target) => Ok(ElementAccess::TupleFieldProjection {
target: Box::new(target),
dot_token,
field,
field_span,
}),
error => error,
},
expr => Err(expr),
}
Expand Down Expand Up @@ -550,4 +553,55 @@ impl Expr {
| Expr::Continue { .. } => false,
}
}

/// Friendly [Expr] name string used for error reporting,
pub fn friendly_name(&self) -> &'static str {
match self {
Expr::Error(_, _) => "error",
Expr::Path(_) => "path",
Expr::Literal(_) => "literal",
Expr::AbiCast { .. } => "ABI cast",
Expr::Struct { .. } => "struct instantiation",
Expr::Tuple(_) => "tuple",
Expr::Parens(_) => "parentheses", // Note the plural!
Expr::Block(_) => "block",
Expr::Array(_) => "array",
Expr::Asm(_) => "assembly block",
Expr::Return { .. } => "return",
Expr::If(_) => "if expression",
Expr::Match { .. } => "match expression",
Expr::While { .. } => "while loop",
Expr::For { .. } => "for loop",
Expr::FuncApp { .. } => "function call",
Expr::Index { .. } => "array element access",
Expr::MethodCall { .. } => "method call",
Expr::FieldProjection { .. } => "struct field access",
Expr::TupleFieldProjection { .. } => "tuple element access",
Expr::Ref { .. } => "referencing",
Expr::Deref { .. } => "dereferencing",
Expr::Not { .. } => "negation",
Expr::Mul { .. } => "multiplication",
Expr::Div { .. } => "division",
Expr::Pow { .. } => "power operation",
Expr::Modulo { .. } => "modulo operation",
Expr::Add { .. } => "addition",
Expr::Sub { .. } => "subtraction",
Expr::Shl { .. } => "left shift",
Expr::Shr { .. } => "right shift",
Expr::BitAnd { .. } => "bitwise and",
Expr::BitXor { .. } => "bitwise xor",
Expr::BitOr { .. } => "bitwise or",
Expr::Equal { .. } => "equality",
Expr::NotEqual { .. } => "non equality",
Expr::LessThan { .. } => "less than operation",
Expr::GreaterThan { .. } => "greater than operation",
Expr::LessThanEq { .. } => "less than or equal operation",
Expr::GreaterThanEq { .. } => "greater than or equal operation",
Expr::LogicalAnd { .. } => "logical and",
Expr::LogicalOr { .. } => "logical or",
Expr::Reassignment { .. } => "reassignment",
Expr::Break { .. } => "break",
Expr::Continue { .. } => "continue",
}
}
}
2 changes: 1 addition & 1 deletion sway-core/src/asm_generation/fuel/fuel_asm_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1788,7 +1788,7 @@ impl<'ir, 'eng> FuelAsmBuilder<'ir, 'eng> {

// ---------------------------------------------------------------------------------------------

// XXX reassess all the places we use this
// TODO-IG: Reassess all the places we use `is_copy_type`.
pub(crate) fn is_copy_type(&self, ty: &Type) -> bool {
ty.is_unit(self.context)
|| ty.is_never(self.context)
Expand Down
59 changes: 48 additions & 11 deletions sway-core/src/control_flow_analysis/dead_code_analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ use crate::{
language::{
parsed::TreeType,
ty::{
self, ConstantDecl, FunctionDecl, StructDecl, TraitDecl, TyAstNode, TyAstNodeContent,
TyDecl, TyImplItem, TypeAliasDecl,
self, ConstantDecl, FunctionDecl, ProjectionKind, StructDecl, TraitDecl, TyAstNode,
TyAstNodeContent, TyDecl, TyImplItem, TypeAliasDecl,
},
CallPath, Visibility,
},
Expand Down Expand Up @@ -1956,23 +1956,60 @@ fn connect_expression<'eng: 'cfg, 'cfg>(
Ok(vec![])
}
Reassignment(typed_reassignment) => {
if let Some(variable_entry) = graph
.namespace
.get_variable(&typed_reassignment.lhs_base_name)
{
for leaf in leaves {
graph.add_edge(*leaf, variable_entry.variable_decl_ix, "".into());
match &typed_reassignment.lhs {
ty::TyReassignmentTarget::ElementAccess {
base_name, indices, ..
} => {
if let Some(variable_entry) = graph.namespace.get_variable(base_name) {
for leaf in leaves {
graph.add_edge(
*leaf,
variable_entry.variable_decl_ix,
"variable reassignment LHS".into(),
);
}
};

for projection in indices {
if let ProjectionKind::ArrayIndex { index, index_span } = projection {
connect_expression(
engines,
&index.expression,
graph,
leaves,
exit_node,
"variable reassignment LHS array index",
tree_type,
index_span.clone(),
options,
)?;
}
}
}
}
ty::TyReassignmentTarget::Deref(exp) => {
connect_expression(
engines,
&exp.expression,
graph,
leaves,
exit_node,
"variable reassignment LHS dereferencing",
tree_type,
exp.span.clone(),
options,
)?;
}
};

connect_expression(
engines,
&typed_reassignment.rhs.expression,
graph,
leaves,
exit_node,
"variable reassignment",
"variable reassignment RHS",
tree_type,
typed_reassignment.rhs.clone().span,
typed_reassignment.rhs.span.clone(),
options,
)
}
Expand Down
Loading

0 comments on commit daee1c2

Please sign in to comment.