[PATCH] D31406: [clang-tidy] Reuse FileID in getLocation

[Alexander Kornienko via Phabricator via cfe-commits]

alexfh requested changes to this revision.
alexfh added a comment.
This revision now requires changes to proceed.

Thank you for finding out the root cause here!



================
Comment at: clang-tidy/ClangTidy.cpp:241
 
-    const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
-    FileID ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
+    if (Path2FileID.find(FilePath) == Path2FileID.end()) {
+      const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
----------------
Repeating lookup three times is not necessary:

  FileID &ID = Path2FileID[FilePath];
  if (!ID.isValid()) {
    const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
    ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
  }

But even this seems to be unnecessary, since SourceManager provides a better API that we should have used here:

  FileID ID = SourceMgr.getOrCreateFileID(File, SrcMgr::C_User);


================
Comment at: clang-tidy/ClangTidy.cpp:256
 
+  DenseMap<StringRef, FileID> Path2FileID;
   FileManager Files;
----------------
`llvm::StringMap<FileID>` would be a better container, if we needed it (see the comment above).


https://reviews.llvm.org/D31406