vec: with_capacity: check for overflow

[emberian]

Fixes #10271

[huonw]

Should this be a failure, or an abort? (Also, does the managed branch need checks too?)

[emberian]

reserve doesn't need checks, see line 1414. reserve_raw in at_vec also checks for overflow.

[huonw]

Isn't this check incorrect, because the header is included in alloc?

[emberian]

No. The header comes from size_of::<Vec<()>>. This is the same code used
for overflow handling elsewhere. cc @thestinger

On Mon, Nov 11, 2013 at 6:23 AM, Huon Wilson notifications@gh.neting.ccwrote:

In src/libstd/vec.rs:

@@ -186,7 +186,11 @@ pub fn with_capacity(capacity: uint) -> ~[T] {
vec
} else {
let alloc = capacity * mem::nonzero_size_of::();

•        let ptr = malloc_raw(alloc + mem::size_of::<Vec<()>>()) as *mut Vec<()>;
  

•        let size = alloc + mem::size_of::<Vec<()>>();
  

•        if alloc / mem::nonzero_size_of::<T>() != capacity || size < alloc {
  

Isn't this check incorrect, because the header is included in alloc?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/10417/files#r7556668
.

[huonw]

Oh, of course... it's alloc / ... not size / ....

[bill-myers]

@cmr Division is slow (it may take 20-100x the time of other arithmetic ops depending on CPU), so it seems a bad idea to use it.

Why not just use checked_mul() and then checked_add() which are generally far faster, and also make the intent totally clear?

[emberian]

@thestinger ? I imagine it's because rust simply didn't have checked_*

On Mon, Nov 11, 2013 at 8:39 AM, bill-myers notifications@gh.neting.ccwrote:

@cmr https://github.com/cmr Division is slow (it may take 20-100x the
time of other arithmetic ops depending on CPU), so it seems a bad idea to
use it.

Why not just use checked_add() and checked_mul(), which are generally far
faster?

—
Reply to this email directly or view it on GitHubhttps://github.com//pull/10417#issuecomment-28200053
.

[huonw]

@bill-myers the division is by a compile-time constant, so it will almost certainly be optimised to some combination of a shift and a multiplication (but only when optimisations are on).

[bill-myers]

@huonw It's still going to be less efficient, since optimizing a division by non-power-of-two constant to a multiplication is not that easy (that is, you need a multiplication by multiplicative inverse and a loop, or an extending multiplication, shifts and other tricks); with checked_mul() you just check the carry flag or that the high part of extended multiplication is 0

[huonw]

@bill-myers that is true (i.e. it becomes a short series of multiplications + shifts... in fact, just checking now, LLVM optimises n / 3 to a single multiplication with a single shift), but the context here is the following operation is either an allocation or task failure, either of which are relatively expensive (and optimising in the case of task failure is certainly not relevant).