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

feat: struct destructuring #2243

Merged
merged 38 commits into from
Jul 8, 2022
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
115efc3
test: struct destructuring
matt-user Jul 1, 2022
bcec9bf
feat: add print statements for debugging
matt-user Jul 1, 2022
58dfcee
debugging: add print
matt-user Jul 1, 2022
9ae75e5
Merge branch 'mattauer/struct_destructuring' of https://github.com/Fu…
matt-user Jul 1, 2022
cb889fb
Merge remote-tracking branch 'origin/master' into mattauer/struct_des…
Jul 1, 2022
483d18b
Add Forc.lock.
Jul 1, 2022
74188c1
Add descriptions.
Jul 1, 2022
e06e04b
fix: remove println! statements
matt-user Jul 5, 2022
df1d3d5
chore: update to latest version
matt-user Jul 5, 2022
69aeccc
Merge branch 'mattauer/struct_destructuring' of https://github.com/Fu…
matt-user Jul 5, 2022
2af43ac
feat: add not working Pattern:Struct branch to match
matt-user Jul 5, 2022
204a251
fix: identation in oracle json
matt-user Jul 6, 2022
bd01ad5
feat: create pattern if field has none
matt-user Jul 6, 2022
60eaa9b
refactor: remove debug prints
matt-user Jul 6, 2022
d715323
fix: warnings
matt-user Jul 6, 2022
06292e8
test: type annotations for destructure
matt-user Jul 6, 2022
9f91f64
fix: handle type annotations
matt-user Jul 6, 2022
8e159fd
chore: run formatter
matt-user Jul 6, 2022
22c76dd
fix: remove test cargo toml
matt-user Jul 6, 2022
4cad09a
Merge branch 'master' into mattauer/struct_destructuring
matt-user Jul 6, 2022
60adadb
fix: destructure test case
matt-user Jul 6, 2022
5d8b0c3
test: nested struct destructuring
matt-user Jul 6, 2022
9d6b86a
docs: add struct destructuring
matt-user Jul 6, 2022
0730cb6
refactor: fix formatting
matt-user Jul 6, 2022
32fcea7
fix: formatting
matt-user Jul 6, 2022
5350b4c
refactor examples
matt-user Jul 6, 2022
f614df8
fix: formatting
matt-user Jul 6, 2022
3904206
fix: remove mention of deleted file
matt-user Jul 6, 2022
1a5b93e
test: nested tuple in struct and vis versa
matt-user Jul 7, 2022
11c5567
docs: add nested tuple in struct and vis versa to examples
matt-user Jul 7, 2022
b48937a
fix: formatting
matt-user Jul 7, 2022
802009d
fix: formatting
matt-user Jul 7, 2022
b9a0e6a
fix: formatting
matt-user Jul 7, 2022
7cd5ad6
fix: formatting
matt-user Jul 7, 2022
9139e18
Merge branch 'master' into mattauer/struct_destructuring
matt-user Jul 7, 2022
12b6f35
Merge branch 'mattauer/struct_destructuring' of https://github.com/Fu…
matt-user Jul 7, 2022
6575499
Merge branch 'master' into mattauer/struct_destructuring
matt-user Jul 7, 2022
fd69348
Merge branch 'master' into mattauer/struct_destructuring
otrho Jul 8, 2022
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
3 changes: 3 additions & 0 deletions sway-core/src/constants.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,9 @@ pub const DEFAULT_ENTRY_POINT_FN_NAME: &str = "main";
/// The default prefix for the compiler generated names of tuples
pub const TUPLE_NAME_PREFIX: &str = "__tuple_";

// The default prefix for the compiler generated names of struct fields
pub const DESTRUCTURE_PREFIX: &str = "__destructure_";

/// The default prefix for the compiler generated names of match
pub const MATCH_RETURN_VAR_NAME_PREFIX: &str = "__match_return_var_name_";

Expand Down
117 changes: 109 additions & 8 deletions sway-core/src/convert_parse_tree.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2531,25 +2531,112 @@ fn statement_let_to_ast_nodes(
let error = ConvertParseTreeError::ConstructorPatternsNotSupportedHere { span };
return Err(ec.error(error));
}
Pattern::Struct { .. } => {
let error = ConvertParseTreeError::StructPatternsNotSupportedHere { span };
return Err(ec.error(error));
Pattern::Struct { fields, .. } => {
let mut ast_nodes = Vec::new();

// Generate a deterministic name for the destructured struct
// Because the parser is single threaded, the name generated below will be stable.
static COUNTER: AtomicUsize = AtomicUsize::new(0);
let destructured_name = format!(
"{}{}",
crate::constants::DESTRUCTURE_PREFIX,
COUNTER.load(Ordering::SeqCst)
);
COUNTER.fetch_add(1, Ordering::SeqCst);
let destructure_name = Ident::new_with_override(
Box::leak(destructured_name.into_boxed_str()),
span.clone(),
);

// Parse the type ascription and the type ascription span.
// In the event that the user did not provide a type ascription,
// it is set to TypeInfo::Unknown and the span to None.
let (type_ascription, type_ascription_span) = match &ty_opt {
Some(ty) => {
let type_ascription_span = ty.span();
let type_ascription = ty_to_type_info(ec, ty.clone())?;
(type_ascription, Some(type_ascription_span))
}
None => (TypeInfo::Unknown, None),
};

// Save the destructure to the new name as a new variable declaration
let save_body_first = VariableDeclaration {
name: destructure_name.clone(),
type_ascription,
type_ascription_span,
body: expression,
is_mutable: false,
};
ast_nodes.push(AstNode {
content: AstNodeContent::Declaration(Declaration::VariableDeclaration(
save_body_first,
)),
span: span.clone(),
});

// create a new variable expression that points to the new destructured struct name that we just created
let new_expr = Expression::VariableExpression {
name: destructure_name,
span: span.clone(),
};

// for all of the fields of the struct destructuring on the LHS,
// recursively create variable declarations
for pattern_struct_field in fields.into_inner().into_iter() {
let (field, recursive_pattern) = match pattern_struct_field {
PatternStructField::Field {
field_name,
pattern_opt,
} => {
let recursive_pattern = match pattern_opt {
Some((_colon_token, box_pattern)) => *box_pattern,
None => Pattern::Var {
mutable: None,
name: field_name.clone(),
},
};
(field_name, recursive_pattern)
}
PatternStructField::Rest { .. } => {
continue;
}
};

// recursively create variable declarations for the subpatterns on the LHS
// and add them to the ast nodes
ast_nodes.extend(unfold(
ec,
recursive_pattern,
None,
Expression::SubfieldExpression {
prefix: Box::new(new_expr.clone()),
span: span.clone(),
field_to_access: field,
},
span.clone(),
)?);
}
ast_nodes
}
Pattern::Tuple(pat_tuple) => {
let mut ast_nodes = Vec::new();

// Generate a deterministic name for the tuple. Because the parser is single
// threaded, the name generated below will be stable.
// Generate a deterministic name for the tuple.
// Because the parser is single threaded, the name generated below will be stable.
static COUNTER: AtomicUsize = AtomicUsize::new(0);
let tuple_name = format!(
"{}{}",
crate::constants::TUPLE_NAME_PREFIX,
COUNTER.load(Ordering::SeqCst)
);
COUNTER.fetch_add(1, Ordering::SeqCst);
let name =
let tuple_name =
Ident::new_with_override(Box::leak(tuple_name.into_boxed_str()), span.clone());

// Parse the type ascription and the type ascription span.
// In the event that the user did not provide a type ascription,
// it is set to TypeInfo::Unknown and the span to None.
let (type_ascription, type_ascription_span) = match &ty_opt {
Some(ty) => {
let type_ascription_span = ty.span();
Expand All @@ -2558,8 +2645,10 @@ fn statement_let_to_ast_nodes(
}
None => (TypeInfo::Unknown, None),
};

// Save the tuple to the new name as a new variable declaration.
let save_body_first = VariableDeclaration {
name: name.clone(),
name: tuple_name.clone(),
type_ascription,
type_ascription_span,
body: expression,
Expand All @@ -2571,19 +2660,31 @@ fn statement_let_to_ast_nodes(
)),
span: span.clone(),
});

// create a variable expression that points to the new tuple name that we just created
let new_expr = Expression::VariableExpression {
name,
name: tuple_name,
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 {
Some(Ty::Tuple(tys)) => Some(tys.into_inner().to_tys()),
_ => None,
};

Copy link
Contributor

Choose a reason for hiding this comment

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

thanks for these comments, great addition

// for all of the elements in the tuple destructuring on the LHS,
// recursively create variable declarations
for (index, pattern) in pat_tuple.into_inner().into_iter().enumerate() {
// from the possible type annotation, grab the type at the index of the current element
// we are processing
let ty_opt = match &tuple_tys_opt {
Some(tys) => tys.get(index).cloned(),
None => None,
};
// recursively create variable declarations for the subpatterns on the LHS
// and add them to the ast nodes
ast_nodes.extend(unfold(
ec,
pattern,
Expand Down
1 change: 0 additions & 1 deletion sway-parse/src/item/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -296,7 +296,6 @@ mod tests {
match Item::parse(&mut parser) {
Ok(item) => item,
Err(_) => {
//println!("Tokens: {:?}", token_stream);
adlerjohn marked this conversation as resolved.
Show resolved Hide resolved
panic!("Parse error: {:?}", errors);
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[[package]]
name = 'core'
source = 'path+from-root-F93ECA3248F311E3'
dependencies = []

[[package]]
name = 'struct_destructuring'
source = 'root'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <contact@fuel.sh>"]
entry = "main.sw"
license = "Apache-2.0"
name = "struct_destructuring"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[
{
"inputs": [],
"name": "main",
"outputs": [
{
"components": null,
"name": "",
"type": "u64",
"typeArguments": null
}
],
"type": "function"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
script;

// Tests nested struct destructuring

fn main() -> u64 {
let point1 = Point { x: 0, y: 0 };
let point2 = Point { x: 1, y: 1 };
let line = Line { p1: point1, p2: point2 };
let Line { p1: Point { x: x0, y: y0 }, p2: Point { x: x1, y: y1} } = line;
Copy link
Contributor

Choose a reason for hiding this comment

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

alright, don't hate me, but what about a nested tuple in a struct and vice versa?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I can add a test for that. Should I also include an example for the docs?

Copy link
Contributor

Choose a reason for hiding this comment

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

Docs example could be cool, to show people just how deep destructuring can go!

x0
}

struct Point {
x: u64,
y: u64,
}

struct Line {
p1: Point,
p2: Point,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "run"
expected_result = { action = "return", value = 0 }
validate_abi = true
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
[[package]]
name = 'core'
source = 'path+from-root-F93ECA3248F311E3'
dependencies = []

[[package]]
name = 'struct_destructuring'
source = 'root'
dependencies = ['core']
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
[project]
authors = ["Fuel Labs <contact@fuel.sh>"]
entry = "main.sw"
license = "Apache-2.0"
name = "struct_destructuring"

[dependencies]
core = { path = "../../../../../../../sway-lib-core" }
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
[
{
"inputs": [],
"name": "main",
"outputs": [
{
"components": null,
"name": "",
"type": "u64",
"typeArguments": null
}
],
"type": "function"
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
script;

// Tests struct destructuring

fn gimme_a_struct() -> Dummy {
Dummy { value1: 1, value2: true }
}

fn main() -> u64 {
let Dummy { value1, value2 } = gimme_a_struct();
let Dummy { value1, value2 }: Dummy = gimme_a_struct();
let data = Data {
value: 42,
};
let Data { value }: Data = data;
return value;
}

struct Data {
value: u64,
}

struct Dummy {
value1: u64,
value2: bool,
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
category = "run"
expected_result = { action = "return", value = 42 }
validate_abi = true
Copy link
Contributor

Choose a reason for hiding this comment

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

as this isn't a contract, do we need to validate the abi?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good catch! I'm not sure, there are other test cases in the same folder such as tuple_access that are scripts and validate the abi.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hm, I don't think it is necessary. Cc @otrho for consensus on that.

Copy link
Contributor

Choose a reason for hiding this comment

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

Time for an RFC!

Copy link
Contributor

@mohammadfawaz mohammadfawaz Jul 6, 2022

Choose a reason for hiding this comment

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

We've been validating the ABI for all passing tests including scripts because scripts emit a JSON ABI now. Not sure how important that is though.

Copy link
Contributor

Choose a reason for hiding this comment

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

BTW, all I did was refactor the test harness. This doesn't make me the authority on how or what to test. 😉