This is an archive of the discontinued LLVM Phabricator instance.

Replace platform-dependent `stat` with `llvm::sys::fs::status`. NFC intended.
ClosedPublic

Authored by vsapsai on Oct 15 2019, 4:07 PM.

Diff Detail

Event Timeline

vsapsai created this revision.Oct 15 2019, 4:07 PM

The plan is to instrument llvm::sys::fs::status with ALWAYS_ENABLED_STATITSTIC to be able to catch regressions causing lots of stat calls. That's why replacing current stat calls. And it seems to be a good change regardless of future plans.

sammccall accepted this revision.Oct 16 2019, 1:10 AM

Watching for regressions in stat calls sounds really useful.

This change is trivially equivalent on linux, but the code path is quite different on windows (I have no idea how ::stat works on windows, but our implementation of fs::status does lots of things).
I don't imagine it'll matter here though, and the original commits don't seem to have avoided fs::status on purpose.

This revision is now accepted and ready to land.Oct 16 2019, 1:10 AM

Thanks for the review! If fs::status behaviour is sufficiently different on Windows, it is worth fixing because I believe majority of non-Windows developers expect them to work in the same way.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptOct 16 2019, 12:12 PM