Implement create_dir_all() to operate iteratively instead of recursively

[asder8215]

The current implementation of create_dir_all(...) in std::fs operates recursively. As mentioned in #124309, this could run into a stack overflow with big paths. To avoid this stack overflow issue, this PR implements the method in an iterative manner, preserving the documented behavior of:

Recursively create a directory and all of its parent components if they are missing.
This function is not atomic. If it returns an error, any parent components it was able to create will remain.
If the empty path is passed to this function, it always succeeds without creating any directories.

[rustbot]

r? @tgross35

rustbot has assigned @tgross35.
They will have a look at your PR within the next two weeks and either review your PR or reassign to another reviewer.

Use r? to explicitly pick a reviewer

[workingjubilee]

Why does this need to manipulate a list of uncreated_dirs at all? Can't we just path.ancestors().rev() and start making directories? So what if we try to recreate the root of the filesystem. Aside from being silly, we'll just get io::ErrorKind::AlreadyExists.

[workingjubilee]

🥴 Answer to jubilee: It's not a reversible iterator!!!

[workingjubilee]

jubilee's counterargument to jubilee's counterargument: Okay, so we'd have to use path.ancestors().nth(i) instead, sure? But you get the idea.

[workingjubilee]

Please feel free to question/reject my comment/suggestion if you feel it is incorrect/scope-creeping, I only am bringing it up because I had precisely this thought after wondering why code I had earlier written elsewhere was doing try_exists and then using create_dir_all.

[workingjubilee]

Followup response to jubilee: what about create_dir_all("./somewhere/../../something")?

in the tune of "Khaaan!": RELATIVE PAAATHS!

[asder8215]

Hey jubilee, I took your suggestion and iterate through the Ancestors Iterator. One thing that I did differently however is that instead of using path.ancestors().nth(i) to repeatedly traverse through the iterator up until the uncreated directory, I used a counter and .take() to pre-allocate the exact memory to hold the remaining directories that I need to create and then just use a reversed iterator on that.

Does this approach look good to you?

[workingjubilee]

Yes, allocating once instead of many times sounds like a reasonable improvement in the spirit of my note, so if others are cool with it, then I'm cool! That way it's "just" a classic space vs. time tradeoff and getting the main improvement we're looking for.

[asder8215]

Hi @tgross35, I noticed that you were the assignee for this pull request. I wanted to ask if there is anything I should change with this PR/commits or if everything looks alright?

[tgross35]

I haven't looked at this yet so I'm going to

r? @workingjubilee

who has. (I know it's at your limit so feel free to reroll again or throw it back if you want)

[rustbot]

workingjubilee is currently at their maximum review capacity.
They may take a while to respond.

[tbu-]

This now always allocates something on the heap when there's at least two directories that are created. Probably not worth optimizing away.

View changes since this review

[asder8215]

r? libs

[rustbot]

Reminder, once the PR becomes ready for a review, use @rustbot ready.

[Mark-Simulacrum]

r=me with commits squashed into one. I agree that adding the reverse-ancestors would be out of scope for this PR.

[rustbot]

⚠️ Warning ⚠️

• This PR is based on an upstream commit that is older than 28 days.

  It's recommended to update your branch according to the rustc-dev-guide.

[asder8215]

@Mark-Simulacrum I ended up using git reset to create the combined commits, hope that is alright with you.

[Mark-Simulacrum]

@bors r+

[bors]

📌 Commit 4081c14 has been approved by Mark-Simulacrum

It is now in the queue for this repository.