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

feat!: add support for string slices #1044

Merged
merged 2 commits into from
Jul 21, 2023
Merged

feat!: add support for string slices #1044

merged 2 commits into from
Jul 21, 2023

Conversation

IGI-111
Copy link
Contributor

@IGI-111 IGI-111 commented Jul 17, 2023 •
edited

Add support for str in prevision for FuelLabs/sway#4778

Checklist

  • I have linked to any relevant issues.
  • I have updated the documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have added necessary labels.
  • I have done my best to ensure that my PR adheres to the Fuel Labs Code Review Standards.
  • I have requested a review from the relevant team or maintainers.

@IGI-111 IGI-111 added enhancement New feature or request breaking Introduces or requires breaking changes labels Jul 17, 2023
@IGI-111 IGI-111 requested a review from a team July 17, 2023 08:26
@IGI-111 IGI-111 self-assigned this Jul 17, 2023
@IGI-111 IGI-111 force-pushed the IGI-111/string-slices branch 9 times, most recently from c5b77d2 to 0b8ca60 Compare July 18, 2023 04:25
@digorithm digorithm changed the title feat: add support for string slices feat!: add support for string slices Jul 19, 2023
digorithm
digorithm previously approved these changes Jul 19, 2023
View reviewed changes
Copy link
Member

@digorithm digorithm left a comment

Choose a reason for hiding this comment

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

LGTM -- thanks for taking the time to do this!

Also, we should update this section of the documentation with this new distinction between string slices and arrays. Can be done in this PR or follow-up work.

@iqdecay
Copy link
Contributor

iqdecay commented Jul 19, 2023

How does this interact with/nullify #1042?

@digorithm
Copy link
Member

How does this interact with/nullify #1042?

I was also wondering about this. It seems like your work on #1042 is for a feature that already exists in the language. And what @IGI-111 is working on is for a future feature. Not sure how these two features interact in the compiler, or if one replaces the other.

@IGI-111, let us know your thoughts.

@IGI-111
Copy link
Contributor Author

IGI-111 commented Jul 20, 2023

Having looked at it it doesn't look like it would conflict at all. This is for a new Sway feature and that other PR is for the existing dynamic String.
One thing that might change is the memory representation of the String type after str slices are implemented in Sway, but this PR isn't doing that kind of change.

So yeah I don't think there's conflict here, not even duplication of efforts per se.

iqdecay
iqdecay previously approved these changes Jul 20, 2023
View reviewed changes
Copy link
Contributor

@iqdecay iqdecay left a comment

Choose a reason for hiding this comment

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

very neat, thanks a lot!

@IGI-111 IGI-111 dismissed stale reviews from iqdecay and digorithm via f86d297 July 20, 2023 14:55
@IGI-111
feat: add support for string slices
b9db3ac 
Add support for `str` in prevision for FuelLabs/sway#4778
@IGI-111 IGI-111 merged commit e5e3186 into master Jul 21, 2023
33 checks passed
@IGI-111 IGI-111 deleted the IGI-111/string-slices branch July 21, 2023 10:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@digorithm digorithm digorithm approved these changes

@Salka1988 Salka1988 Salka1988 approved these changes

@hal3e hal3e hal3e approved these changes

@MujkicA MujkicA Awaiting requested review from MujkicA MujkicA is a code owner

@Br1ght0ne Br1ght0ne Awaiting requested review from Br1ght0ne Br1ght0ne is a code owner

@segfault-magnet segfault-magnet Awaiting requested review from segfault-magnet segfault-magnet is a code owner

@iqdecay iqdecay Awaiting requested review from iqdecay

Assignees

@IGI-111 IGI-111

Labels
breaking Introduces or requires breaking changes enhancement New feature or request
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
6 participants
@IGI-111 @iqdecay @digorithm @Salka1988 @hal3e @segfault-magnet