This is an archive of the discontinued LLVM Phabricator instance.

[VFS] Combine VFSFromYamlDirIterImpl and OverlayFSDirIterImpl into a single implementation (NFC)
ClosedPublic

Authored by nathawes on Jan 15 2021, 8:34 PM.

Details

Summary

As a fixme notes, both of these directory iterator implementations are conceptually similar and duplicate the functionality of returning and uniquing entries across two or more directories. This patch addresses the fixme by extracting their common functionality into a single class 'CombiningDirIterImpl'.

This also drops the 'Redirecting' prefix from RedirectingDirEntry and RedirectingFileEntry to save horizontal space. There's no loss of clarity as they already have to be prefixed with 'RedirectingFileSystem::' whenever they're referenced anyway.

rdar://problem/72485443

Diff Detail

Event Timeline

nathawes created this revision.Jan 15 2021, 8:34 PM
nathawes requested review of this revision.Jan 15 2021, 8:34 PM
Herald added a project: Restricted Project. · View Herald TranscriptJan 15 2021, 8:34 PM
dexonsmith accepted this revision.Jan 17 2021, 2:52 PM

LGTM! Thanks for splitting it out.

This revision is now accepted and ready to land.Jan 17 2021, 2:52 PM
JDevlieghere accepted this revision.Jan 22 2021, 9:16 AM
JDevlieghere added inline comments.
llvm/include/llvm/Support/VirtualFileSystem.h
661

Nit: most of the implementation uses EC as the variable name for an error_code.

nathawes updated this revision to Diff 320031.Jan 28 2021, 7:36 PM
  • Rename error_code to EC in shouldFallBackToExternalFS() to match existing conventions in the file.
  • Update a missed RedirectingFileEntry reference in lldb to its new name (FileEntry)
Herald added a project: Restricted Project. · View Herald TranscriptJan 28 2021, 7:36 PM
This revision was landed with ongoing or failed builds.Jan 29 2021, 5:13 PM
This revision was automatically updated to reflect the committed changes.