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.

Sign up for GitHub

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

Jump to bottom

Implementing tuple access via desugaring and indexing. #681

Merged
merged 15 commits into from
Feb 2, 2022
Merged

Implementing tuple access via desugaring and indexing. #681

merged 15 commits into from
Feb 2, 2022

Conversation

emilyaherbert
Copy link
Contributor

Closes #632

@emilyaherbert emilyaherbert added enhancement New feature or request compiler General compiler. Should eventually become more specific as the issue is triaged labels Jan 21, 2022
@emilyaherbert emilyaherbert self-assigned this Jan 21, 2022
@emilyaherbert emilyaherbert marked this pull request as ready for review January 24, 2022 23:12
@emilyaherbert emilyaherbert requested a review from sezna as a code owner January 24, 2022 23:12
@emilyaherbert emilyaherbert requested review from otrho, sezna and canndrew and removed request for sezna January 24, 2022 23:12
adlerjohn
adlerjohn reviewed Jan 26, 2022
View reviewed changes
sway-core/Cargo.toml Outdated Show resolved Hide resolved
sezna
sezna suggested changes Jan 28, 2022
View reviewed changes
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

Overall looks good but I think the test coverage needs to be improved. See my comment for some test cases.

sway-core/src/build_config.rs
@@ -10,6 +13,7 @@ pub struct BuildConfig {
pub(crate) print_intermediate_asm: bool,
pub(crate) print_finalized_asm: bool,
pub(crate) print_ir: bool,
pub(crate) generated_names: Arc<Mutex<Vec<&'static str>>>,
Copy link
Contributor

Choose a reason for hiding this comment

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

why this instead of a Arc<Vec<&'static str>>?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Arc<Vec<&'static str>> makes it difficult to push to the vec. There may be another way to accomplish this, but this is the one I am familiar with.

sway-core/src/parse_tree/declaration.rs Show resolved Hide resolved
sway-core/src/parse_tree/declaration/variable.rs Outdated Show resolved Hide resolved
sway-core/src/parse_tree/declaration/variable.rs Show resolved Hide resolved
test/src/e2e_vm_tests/test_programs/tuple_indexing/Forc.toml Outdated Show resolved Hide resolved
test/src/e2e_vm_tests/test_programs/tuple_access/Forc.toml Outdated Show resolved Hide resolved
@emilyaherbert emilyaherbert requested a review from sezna February 1, 2022 00:48
sezna
sezna approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

LGTM, awesome!

@sezna sezna merged commit d723be5 into master Feb 2, 2022
@sezna sezna deleted the emilyaherbert-632/tuple-destructuring-and-indexing branch February 2, 2022 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adlerjohn adlerjohn adlerjohn left review comments

@sezna sezna sezna approved these changes

@otrho otrho Awaiting requested review from otrho

@canndrew canndrew Awaiting requested review from canndrew

Assignees

@emilyaherbert emilyaherbert

Labels
compiler General compiler. Should eventually become more specific as the issue is triaged enhancement New feature or request
Projects
Fuel Network
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Implement tuple access syntax via destructuring and indexing
3 participants
@emilyaherbert @adlerjohn @sezna