This is an archive of the discontinued LLVM Phabricator instance.

NFCI: Simplify SourceManager::translateFile by removing code path that should never be taken
ClosedPublic

Authored by arphaman on Jul 30 2019, 3:31 PM.

Details

Summary

I noticed that SourceManager::translateFile has code that doesn't really make sense. In particular, if it fails to find a FileID by comparing FileEntry * values, it tries to look through files that have the same filename, to see if they have a matching inode to try to find the right FileID. However, the inode comparison seem redundant, as Clang's FileManager already deduplicates FileEntry * values by inode. Thus the comparisons between inodes should never actually succeed, and the comparison between FileEntry * values should be sufficient here.

This observation is supported by the code coverage report that shows that we never actually reach the case where the INODE comparison succeeds:
http://lab.llvm.org:8080/coverage/coverage-reports/clang/coverage/Users/buildslave/jenkins/workspace/clang-stage2-coverage-R/llvm/tools/clang/lib/Basic/SourceManager.cpp.html#L1595

Diff Detail

Repository
rL LLVM

Event Timeline

arphaman created this revision.Jul 30 2019, 3:31 PM
Herald added a project: Restricted Project. · View Herald TranscriptJul 30 2019, 3:31 PM

Ping. I will commit it this week if there are no objections.

This revision was not accepted when it landed; it landed in state Needs Review.Aug 21 2019, 2:36 PM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 21 2019, 2:36 PM