Skip to content

Conversation

@Nokel81
Copy link

@Nokel81 Nokel81 commented Mar 12, 2020

Fixes: #32

@sugar700
Copy link

sugar700 commented Jan 25, 2022

This is unsound. Consider what happens if break is used as an expression. Also, this should be using $crate:: to avoid risk of referring to wrong std.

Comment on lines +181 to +183
for i in 0..$count {
res[i] = ::std::mem::MaybeUninit::new($value);
}
Copy link

Choose a reason for hiding this comment

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

Suggested change
for i in 0..$count {
res[i] = ::std::mem::MaybeUninit::new($value);
}
for elem in &mut res[..] {
elem.write($value);
}

A little cleaner, taken from https://doc.rust-lang.org/stable/std/mem/union.MaybeUninit.html#initializing-an-array-element-by-element

@oaleaf
Copy link

oaleaf commented Feb 12, 2022

Consider what happens if break is used as an expression.

There is no break, is there?

The only issue would be a potential panic! in the body of the loop and the already initialized elements not getting dropped, leading to a memory leak

@sugar700
Copy link

Consider what happens if break is used as an expression.

There is no break, is there?

The only issue would be a potential panic! in the body of the loop and the already initialized elements not getting dropped, leading to a memory leak

Well, the user could provide break as an expression, and that would escape the loop early.

@oaleaf
Copy link

oaleaf commented Feb 13, 2022

Oh I see what you mean now.

Maybe moving the expression to a variable outside the loop and cloning it for every element would be better in that regard?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Macro for slices of clone

3 participants