Skip to content

Migrate to std::filesystem usage in FilePath#2428

Open
borrrden wants to merge 5 commits intomasterfrom
feature/std_filesystem
Open

Migrate to std::filesystem usage in FilePath#2428
borrrden wants to merge 5 commits intomasterfrom
feature/std_filesystem

Conversation

@borrrden
Copy link
Member

@borrrden borrrden commented Jan 30, 2026

This leaves the FilePath interface intact and replaces its implementation with std::filesystem. This is the cleanest way to immediately eliminate legacy code using non standard C APIs to accomplish the same. As a follow up, FilePath could be removed entirely.

@borrrden borrrden requested review from jianminzhao and snej January 30, 2026 23:17
@borrrden
Copy link
Member Author

I forgot to lint 😢 and also Windows is unhappy with some of this for whatever reason....(seems like it doesn't have an implicit conversion like the other compilers to)

@cbl-bot
Copy link

cbl-bot commented Jan 30, 2026

Code Coverage Results:

Type Percentage
branches 64.88
functions 77.22
instantiations 70.92
lines 76.07
regions 72.34

@borrrden
Copy link
Member Author

borrrden commented Feb 3, 2026

There was a Windows PR validation failure but it looks like flake rather than anything related to this PR.

@snej
Copy link
Collaborator

snej commented Feb 3, 2026

Is there a way to keep the FilePath API closer to what it was so there are fewer changes in the codebase?
I have a gut feeling that std::filesystem should remain an internal detail of FilePath until we're ready to replace it entirely. But I'm open to counter-arguments :)

@borrrden
Copy link
Member Author

borrrden commented Feb 3, 2026

Is there a way to keep the FilePath API closer to what it was so there are fewer changes in the codebase?

That's exactly what I did, except for the constructor (and filepath is easily convertible to and from string so think of it as a string constructor). It was really awkward to keep the dir and file separate in this new system because it adds no meaning and forces a lot of extra checks.

For some reason the existing one doesn't provide a good result in some Linux machines.
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.

3 participants