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. _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
