arphaman created this revision.
arphaman added reviewers: rsmith, bruno, Bigcheese.
Herald added subscribers: dexonsmith, jkorous.
Herald added a project: clang.
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
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D65481
Files:
clang/lib/Basic/SourceManager.cpp
Index: clang/lib/Basic/SourceManager.cpp
===================================================================
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -1558,22 +1558,6 @@
// 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 @@
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 @@
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();
+ // 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;
- }
- }
- }
+ 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.
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
