-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
Handle case when Bytes
appends to self
#6308
Conversation
This is not the behaviour id expect personally. Id expect this to duplicate the array.
|
I think there's ambiguity here since appending should both add the elements to the slice it's called on and consume the slice argument it's called with. The combination of these two does lead to the identity function, but it is a surprising result. Rust gets around this problem by making it simply illegal to append onto oneself since that would be a double mutable borrow. C++ is the more similar memory model to us and this: #include <string>
int main() {
std::string str = "abc";
str.append(str);
printf("%s\n", str.c_str());
return 0;
} Does print I'm with @SwayStar123 here that doubling the bytes is the more intuitive result. Though it bares mention in the docs. And we should be clear about what the exact semantics are here. |
I think I would prefer Rust's approach here and make it illegal. |
I agree with @bitzoic that we should follow Rust's approach for |
Making it illegal is not reasonable without a borrow checker. We can't reliably check that you are trying to mutate the same memory twice at once without heavy annotations. Sway is like C++ in that way. We could just panic, but that seems a lot less coherent that picking a correct output for the reflexive call and sticking to it. |
This is what line 696 does. sway/sway-lib-std/src/bytes.sw Line 696 in 2776aff
This is what line 697 does sway/sway-lib-std/src/bytes.sw Line 697 in 2776aff
|
Description
When calling
self.append(self)
forBytes
, we experienced undefined behavior as the logic would clearself
. Now if this case is encountered, the function will revert.Checklist
Breaking*
orNew Feature
labels where relevant.