fix: avoid stack overflow with std::unique_ptr linked list

[ianna]

No description provided.

[codecov]

Codecov Report

Merging #2366 (2749c22) into main (29f89d8) will not change coverage.
The diff coverage is n/a.

Additional details and impacted files

[jpivarski]

I don't know enough modern C++ to be sure that this is right, but it is a for loop, rather than recursion.

The loop termination I understand, though I would have written current; as current != nullptr; for clarity.

I don't see how current = std::move(current->next_) deletes what current is pointing to and sets it to the value of current->next_. This must use some properties of std::unique_ptr somehow...

But it clearly looks like it's addressing the issue with an explicit for loop and if you're not getting giant memory leaks in your billion-value tests, then it must be deleting those panels.

[ianna]

I don't know enough modern C++ to be sure that this is right, but it is a for loop, rather than recursion.

The loop termination I understand, though I would have written current; as current != nullptr; for clarity.

I don't see how current = std::move(current->next_) deletes what current is pointing to and sets it to the value of current->next_. This must use some properties of std::unique_ptr somehow...

The move transfers ownership and nulls it, please, see 20.8.1 in https://isocpp.org/files/papers/n4296.pdf

But it clearly looks like it's addressing the issue with an explicit for loop and if you're not getting giant memory leaks in your billion-value tests, then it must be deleting those panels.

The recursion is gone :-) Please, see a partially expanded stack before the change:

and a fully expanded one after:

[jpivarski]

Okay, good explanation! I'll merge it now.