This is an archive of the discontinued LLVM Phabricator instance.

[clang] Only modify FileEntryRef names that are externally remapped
ClosedPublic

Authored by benlangmuir on Aug 1 2022, 1:37 PM.

Details

Summary

As progress towards having FileEntryRef contain the requested name of the file, this commit narrows the "remap" hack to only apply to paths that were remapped to an external contents path by a VFS. That was always the original intent of this code, and the fact it was making relative paths absolute was an unintended side effect.

Diff Detail

Event Timeline

benlangmuir created this revision.Aug 1 2022, 1:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2022, 1:37 PM
benlangmuir requested review of this revision.Aug 1 2022, 1:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 1 2022, 1:37 PM
Herald added a subscriber: cfe-commits. · View Herald Transcript
bnbarham accepted this revision.Aug 1 2022, 1:57 PM
bnbarham added inline comments.
clang/lib/Basic/FileManager.cpp
277–278

Is it possible for an ExposesExternalVFSPath to have the same filename as the one that was requested? Just in the case of mapping A -> A or something equally pointless? Could check for that in the VFS, but probably doesn't matter in the long run.

This revision is now accepted and ready to land.Aug 1 2022, 1:57 PM
benlangmuir added inline comments.Aug 1 2022, 2:04 PM
clang/lib/Basic/FileManager.cpp
277–278

I think in practice it won't happen, but in theory the VFS can do whatever it wants, so I didn't want to introduce any possibility of circularity here. I will flip these checks so the boolean comes before the string compare though, since that will be cheaper.

Flip the order of comparisons to avoid unnecessary string comparisons.

This revision was landed with ongoing or failed builds.Aug 1 2022, 3:46 PM
This revision was automatically updated to reflect the committed changes.