This is an archive of the discontinued LLVM Phabricator instance.

[clang][lex] NFCI: Use FileEntryRef in ModuleMap::{load,lookup}ModuleMap()
ClosedPublic

Authored by jansvoboda11 on Jun 13 2022, 8:12 AM.

Details

Summary

This patch changes the return/argument types of ModuleMap::{load,lookup}ModuleMap() from const FileEntry * to FileEntryRef in order to remove uses of the deprecated DirectoryEntry::getName().

Diff Detail

Event Timeline

jansvoboda11 created this revision.Jun 13 2022, 8:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2022, 8:12 AM
jansvoboda11 requested review of this revision.Jun 13 2022, 8:12 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2022, 8:12 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
bnbarham accepted this revision.Jun 13 2022, 9:38 AM

Few small comments, LGTM otherwise.

clang/include/clang/Lex/HeaderSearch.h
628–629
clang/lib/Frontend/FrontendAction.cpp
447

The empty case isn't checked as far as I can tell. The null case wasn't either, but seems like we should just return true?

clang/lib/Lex/HeaderSearch.cpp
1718

Nitpick: could just return F directly in the 3 ifs here.

This revision is now accepted and ready to land.Jun 13 2022, 9:38 AM
jansvoboda11 added inline comments.Jun 13 2022, 10:41 AM
clang/lib/Frontend/FrontendAction.cpp
447

Good point. I think the entry for MainFileID always exists, so I'll add an assert here.

The failure here is likely due to the hack in FileManager::getFileRef:

// FIXME: This hack ensures that `getDir()` will use the path that was
// used to lookup this file, even if we found a file by different path
// first. This is required in order to find a module's structure when its
// headers/module map are mapped in the VFS.

So the directory returned from the FileEntry is the *lookup* directory, but the FileEntryRef will be giving back the directory from the external path (if it has been remapped).

TODO: Try rebasing this on top of D151398.

Rebase on top of D151398

jansvoboda11 marked 2 inline comments as done.May 25 2023, 12:55 PM
This revision was landed with ongoing or failed builds.May 30 2023, 1:54 PM
This revision was automatically updated to reflect the committed changes.