Page MenuHomePhabricator

Replace clang::FileData with llvm::vfs::Status
ClosedPublic

Authored by harlanhaskins on Mar 4 2019, 1:58 PM.

Details

Summary

FileData was only ever used as a container for the values in
llvm::vfs::Status, so they might as well be consolidated.

The InPCH member was also always set to false, and unused.

Diff Detail

Repository
rL LLVM

Event Timeline

harlanhaskins created this revision.Mar 4 2019, 1:58 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2019, 1:58 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
harlanhaskins edited the summary of this revision. (Show Details)Mar 4 2019, 1:58 PM
harlanhaskins added a reviewer: benlangmuir.
ormris removed a subscriber: ormris.Mar 4 2019, 2:33 PM
benlangmuir accepted this revision.Mar 4 2019, 6:05 PM

LGTM, although I made a small suggestion for clarity. FYI InPCH was used by PTH, which was removed a couple of months ago.

clang/lib/Basic/FileManager.cpp
305 ↗(On Diff #189202)

Why copy Status back into Status instead of mutating the relevant fields?

This revision is now accepted and ready to land.Mar 4 2019, 6:05 PM
harlanhaskins marked an inline comment as done.Mar 4 2019, 6:07 PM
harlanhaskins added inline comments.
clang/lib/Basic/FileManager.cpp
305 ↗(On Diff #189202)

The fields don't have setters exposed, and I couldn't decide if adding the setters vs. re-constructing a Status was better here. Would it be worth adding setters to the properties in Status?

benlangmuir added inline comments.Mar 4 2019, 6:13 PM
clang/lib/Basic/FileManager.cpp
305 ↗(On Diff #189202)

Nah, just go with the code you already wrote. Thanks for the explanation.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 4 2019, 6:26 PM