modularml / mojo

The Mojo Programming Language
https://docs.modular.com/mojo
Other
22.11k stars 2.54k forks source link

[stdlib] `List.resize(N)` after default-constructed `List` crashes #2938

Open JoeLoser opened 1 month ago

JoeLoser commented 1 month ago
from collections import List

fn main():
    var res = List[Int]()
    res.resize(5) # crashes

crashes. We should investigate and fix this. The other overload of resize(N, value) does not crash, e.g.

from collections import List

fn main():
    var res = List[Int]()
    res.resize(5, 0) # does not crash
gabrieldemarmiesse commented 1 month ago

Reference is here: https://docs.modular.com/mojo/stdlib/collections/list/List#resize

With no new value provided, the new size must be smaller than or equal to the current one. Elements at the end are discarded.

So it's not really a bug, it's the expected behavior.

So I believe we should make sure that the new size is smaller and abort if it's not the case, that would replace an ugly crash by a helpful crash. Would that work for you?

JoeLoser commented 1 month ago

Reference is here: https://docs.modular.com/mojo/stdlib/collections/list/List#resize

With no new value provided, the new size must be smaller than or equal to the current one. Elements at the end are discarded.

So it's not really a bug, it's the expected behavior.

So I believe we should make sure that the new size is smaller and abort if it's not the case, that would replace an ugly crash by a helpful crash. Would that work for you?

Yep, aborting on that sounds good to me. FYI @abduld as you ran into this earlier today.