Skip to content

Commit

Permalink
Error out on wrongly sized tuple destructuring (FuelLabs#4281)
Browse files Browse the repository at this point in the history
## Description

Fix FuelLabs#4270

Changed the way we convert the parse tree for tuple destructuring to add
an annotation of a tuple of the proper size of placeholder types.

e.g.:

```sway
let (a, b): (bool, u64) = c;
```

now gets desugared to something like

```
let __tuple: (bool, u64) = c
let __tuple: (_, _) = __tuple
let a: bool = __tuple.0;
let b: u64 = __tuple.1;
```

In the bad case (if `c` is something like `(bool, u64, u64)` it would no
longer type check for lack of being unifiable with `(_, _)`.

This required a small modification to the unification check to allow
placeholders on the right side.

## 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).
- [x] I have added tests that prove my fix is effective or that my
feature works.
- [x] 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
IGI-111 authored Mar 14, 2023
1 parent 6794228 commit 9239e6e
Show file tree
Hide file tree
Showing 8 changed files with 89 additions and 2 deletions.
62 changes: 61 additions & 1 deletion sway-core/src/transform/to_parsed_lang/convert_parse_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3110,12 +3110,72 @@ fn statement_let_to_ast_nodes(
span: span.clone(),
});

// Acript a second declaration to a tuple of placeholders to check that the tuple
// is properly sized to the pattern
let placeholders_type_ascription = {
let type_id = engines.te().insert(
engines.de(),
TypeInfo::Tuple(
pat_tuple
.clone()
.into_inner()
.into_iter()
.map(|_| {
let initial_type_id =
engines.te().insert(engines.de(), TypeInfo::Unknown);
let dummy_type_param = TypeParameter {
type_id: initial_type_id,
initial_type_id,
name_ident: Ident::new_with_override(
"_".into(),
span.clone(),
),
trait_constraints: vec![],
trait_constraints_span: Span::dummy(),
};
let initial_type_id = engines.te().insert(
engines.de(),
TypeInfo::Placeholder(dummy_type_param),
);
TypeArgument {
type_id: initial_type_id,
initial_type_id,
call_path_tree: None,
span: Span::dummy(),
}
})
.collect(),
),
);
TypeArgument {
type_id,
initial_type_id: type_id,
span: tuple_name.span(),
call_path_tree: None,
}
};

// create a variable expression that points to the new tuple name that we just created
let new_expr = Expression {
kind: ExpressionKind::Variable(tuple_name),
kind: ExpressionKind::Variable(tuple_name.clone()),
span: span.clone(),
};

// Override the previous declaration with a tuple of placeholders to check the
// shape of the tuple
let check_tuple_shape_second = VariableDeclaration {
name: tuple_name,
type_ascription: placeholders_type_ascription,
body: new_expr.clone(),
is_mutable: false,
};
ast_nodes.push(AstNode {
content: AstNodeContent::Declaration(Declaration::VariableDeclaration(
check_tuple_shape_second,
)),
span: span.clone(),
});

// from the possible type annotation, if the annotation was a tuple annotation,
// extract the internal types of the annotation
let tuple_tys_opt = match ty_opt {
Expand Down
3 changes: 3 additions & 0 deletions sway-core/src/type_system/unify_check.rs
Original file line number Diff line number Diff line change
Expand Up @@ -360,6 +360,9 @@ impl<'a> UnifyCheck<'a> {
for j in (i + 1)..right_types.len() {
let a = right_types.get(i).unwrap();
let b = right_types.get(j).unwrap();
if matches!(a, Placeholder(_)) || matches!(b, Placeholder(_)) {
continue;
}
if a.eq(b, self.engines) {
// if a and b are the same type
constraints.push((i, j));
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
out
target
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
[[package]]
name = 'wrongly_sized_tuple_destructuring'
source = 'member'
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[project]
authors = ["Fuel Labs <[email protected]>"]
entry = "main.sw"
license = "Apache-2.0"
name = "wrongly_sized_tuple_destructuring"
implicit-std = false
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
script;

fn main() {
let a = (true, 10, 64);
let (b, c) = a;
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
category = "fail"

# check: let (b, c) = a;
# nextln: $()Mismatched types.
# nextln: $()expected: (_, _)
# nextln: $()found: (bool, u64, u64).
# nextln: $()help: Variable declaration's type annotation does not match up with the assigned expression's type.
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ script;
use basic_storage_abi::{BasicStorage, Quad};

fn main() -> u64 {
let addr = abi(BasicStorage, 0xab8a772238071362b952208cbca054869a45eda6107376fe8c50a44e8ca284f1);
let addr = abi(BasicStorage, 0x8a8effaea73f7ab95bd1656a33b6c0cc36c70c907ab7387fd0b9860efc756a5e);
let key = 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff;
let value = 4242;

Expand Down

0 comments on commit 9239e6e

Please sign in to comment.