r369585 - NFCI: Simplify SourceManager::translateFile by removing code path that should never be taken
Alex Lorenz via cfe-commits
cfe-commits at lists.llvm.org
Wed Aug 21 14:37:09 PDT 2019
Author: arphaman
Date: Wed Aug 21 14:37:09 2019
New Revision: 369585
URL: http://llvm.org/viewvc/llvm-project?rev=369585&view=rev
Log:
NFCI: Simplify SourceManager::translateFile by removing code path that should never be taken
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.
Differential Revision: https://reviews.llvm.org/D65481
Modified:
cfe/trunk/lib/Basic/SourceManager.cpp
Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=369585&r1=369584&r2=369585&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Wed Aug 21 14:37:09 2019
@@ -1558,22 +1558,6 @@ unsigned SourceManager::getFileIDSize(Fi
// Other miscellaneous methods.
//===----------------------------------------------------------------------===//
-/// Retrieve the inode for the given file entry, if possible.
-///
-/// This routine involves a system call, and therefore should only be used
-/// in non-performance-critical code.
-static Optional<llvm::sys::fs::UniqueID>
-getActualFileUID(const FileEntry *File) {
- if (!File)
- return None;
-
- llvm::sys::fs::UniqueID ID;
- if (llvm::sys::fs::getUniqueID(File->getName(), ID))
- return None;
-
- return ID;
-}
-
/// Get the source location for the given file:line:col triplet.
///
/// If the source file is included multiple times, the source location will
@@ -1595,13 +1579,8 @@ SourceLocation SourceManager::translateF
FileID SourceManager::translateFile(const FileEntry *SourceFile) const {
assert(SourceFile && "Null source file!");
- // Find the first file ID that corresponds to the given file.
- FileID FirstFID;
-
// First, check the main file ID, since it is common to look for a
// location in the main file.
- Optional<llvm::sys::fs::UniqueID> SourceFileUID;
- Optional<StringRef> SourceFileName;
if (MainFileID.isValid()) {
bool Invalid = false;
const SLocEntry &MainSLoc = getSLocEntry(MainFileID, &Invalid);
@@ -1609,100 +1588,35 @@ FileID SourceManager::translateFile(cons
return FileID();
if (MainSLoc.isFile()) {
- const ContentCache *MainContentCache
- = MainSLoc.getFile().getContentCache();
- if (!MainContentCache || !MainContentCache->OrigEntry) {
- // Can't do anything
- } else if (MainContentCache->OrigEntry == SourceFile) {
- FirstFID = MainFileID;
- } else {
- // Fall back: check whether we have the same base name and inode
- // as the main file.
- const FileEntry *MainFile = MainContentCache->OrigEntry;
- SourceFileName = llvm::sys::path::filename(SourceFile->getName());
- if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) {
- SourceFileUID = getActualFileUID(SourceFile);
- if (SourceFileUID) {
- if (Optional<llvm::sys::fs::UniqueID> MainFileUID =
- getActualFileUID(MainFile)) {
- if (*SourceFileUID == *MainFileUID) {
- FirstFID = MainFileID;
- SourceFile = MainFile;
- }
- }
- }
- }
- }
+ const ContentCache *MainContentCache =
+ MainSLoc.getFile().getContentCache();
+ if (MainContentCache && MainContentCache->OrigEntry == SourceFile)
+ return MainFileID;
}
}
- if (FirstFID.isInvalid()) {
- // The location we're looking for isn't in the main file; look
- // through all of the local source locations.
- for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
- bool Invalid = false;
- const SLocEntry &SLoc = getLocalSLocEntry(I, &Invalid);
- if (Invalid)
- return FileID();
-
- if (SLoc.isFile() &&
- SLoc.getFile().getContentCache() &&
- SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
- FirstFID = FileID::get(I);
- break;
- }
- }
- // If that still didn't help, try the modules.
- if (FirstFID.isInvalid()) {
- for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
- const SLocEntry &SLoc = getLoadedSLocEntry(I);
- if (SLoc.isFile() &&
- SLoc.getFile().getContentCache() &&
- SLoc.getFile().getContentCache()->OrigEntry == SourceFile) {
- FirstFID = FileID::get(-int(I) - 2);
- break;
- }
- }
- }
+ // The location we're looking for isn't in the main file; look
+ // through all of the local source locations.
+ for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
+ bool Invalid = false;
+ const SLocEntry &SLoc = getLocalSLocEntry(I, &Invalid);
+ if (Invalid)
+ return FileID();
+
+ if (SLoc.isFile() && SLoc.getFile().getContentCache() &&
+ SLoc.getFile().getContentCache()->OrigEntry == SourceFile)
+ return FileID::get(I);
}
- // If we haven't found what we want yet, try again, but this time stat()
- // each of the files in case the files have changed since we originally
- // parsed the file.
- if (FirstFID.isInvalid() &&
- (SourceFileName ||
- (SourceFileName = llvm::sys::path::filename(SourceFile->getName()))) &&
- (SourceFileUID || (SourceFileUID = getActualFileUID(SourceFile)))) {
- bool Invalid = false;
- for (unsigned I = 0, N = local_sloc_entry_size(); I != N; ++I) {
- FileID IFileID;
- IFileID.ID = I;
- const SLocEntry &SLoc = getSLocEntry(IFileID, &Invalid);
- if (Invalid)
- return FileID();
-
- if (SLoc.isFile()) {
- const ContentCache *FileContentCache
- = SLoc.getFile().getContentCache();
- const FileEntry *Entry = FileContentCache ? FileContentCache->OrigEntry
- : nullptr;
- if (Entry &&
- *SourceFileName == llvm::sys::path::filename(Entry->getName())) {
- if (Optional<llvm::sys::fs::UniqueID> EntryUID =
- getActualFileUID(Entry)) {
- if (*SourceFileUID == *EntryUID) {
- FirstFID = FileID::get(I);
- SourceFile = Entry;
- break;
- }
- }
- }
- }
- }
+ // If that still didn't help, try the modules.
+ for (unsigned I = 0, N = loaded_sloc_entry_size(); I != N; ++I) {
+ const SLocEntry &SLoc = getLoadedSLocEntry(I);
+ if (SLoc.isFile() && SLoc.getFile().getContentCache() &&
+ SLoc.getFile().getContentCache()->OrigEntry == SourceFile)
+ return FileID::get(-int(I) - 2);
}
- (void) SourceFile;
- return FirstFID;
+ return FileID();
}
/// Get the source location in \arg FID for the given line:col.
More information about the cfe-commits
mailing list