-
Notifications
You must be signed in to change notification settings - Fork 5.4k
Vec implementation using slices #6933
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
base: master
Are you sure you want to change the base?
Conversation
CodSpeed Performance ReportMerging #6933 will not alter performanceComparing Summary
|
## Description This PR is part of #5110. It fixes a problem that improves the usability of slices. When a method is returned from the "method resolution algorithm", if any generic is involved, it is returned as its generic form. So for example, if a method `len` is called on `Vec<u8>`, we return something like `fn len(self: Vec<T>)`. Later we monomorphize `T` into `u8`. To do that we run a "pattern match" to realize that we need to replace `T` with `u8`. In this case, the bug was that we were not considering that some primitive types have "implicit type parameters". This was found whilst creating more tests for #6933, because I was not able to call the `len` method defined for slices as: ```sway impl<T> &[T] { pub fn len(self) -> u64 { ... } } ``` ## Checklist - [x] I have linked to any relevant issues. - [x] I have commented my code, particularly in hard-to-understand areas. - [ ] 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. - [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.
dfd6ba0
to
ea54b06
Compare
d541d91
to
b328dac
Compare
d7445aa
to
97a6d26
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, just one nit
@@ -1,3 +1,2 @@ | |||
category = "run" | |||
expected_result = { action = "return_data", value = "0000000003ffffc800000000000000040000000000000003" } | |||
expected_result_new_encoding = { action = "return_data", value = "0000000000000003000000000000007c000000000000007c000000000000007c" } | |||
expected_result_new_encoding = { action = "return_data", value = "0000000000000003 000000000000007c 000000000000007c 000000000000007c" } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious why expected_result
is removed
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now that "new_encoding" is enabled by default, it is only used if the old encoding is explicitly asked to be used.
9e21b02
to
dba0a56
Compare
447cfd4
to
e6dd95f
Compare
Description
This PR is a POC of using slices to implement
Vec<T>
.Checklist
Breaking*
orNew Feature
labels where relevant.