[clang-tidy] Reuse FileID in getLocation

[chh]

Summary

http://bugs.llvm.org/show_bug.cgi?id=32402
One FileID per warning will increase and overflow NextLocalOffset
when input file is large with many warnings.
Reusing FileID avoids this problem.

[xazax.hun]

clang-tidy/ClangTidy.cpp

246 ↗

(On Diff #93179)

I do not know how important the efficiency here. But you do not need to store strings (and allocate buffers). You could get the FileEntry from the FileID and query the name (which will give you a StringRef).

After that modification, StringRef is relatively small, so maybe the map is a good candidate to be a DenseMap.

[chh]

Use DenseMap<StringRef, FileID>.

[chh]

clang-tidy/ClangTidy.cpp

246 ↗

(On Diff #93179)

Now replaced with DenseMap<StringRef, FileID>.

[alexfh]

Thank you for finding out the root cause here!

clang-tidy/ClangTidy.cpp

241 ↗

(On Diff #93284)

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);

256 ↗

(On Diff #93284)

llvm::StringMap<FileID> would be a better container, if we needed it (see the comment above).

[chh]

Use getOrCreateFileID.

[alexfh]

LG
Do you have commit rights?

[chh]

This was reverted due to one failed test case clang-tidy/llvm-include-order.cpp on Windows:

Assertion failed: EndColNo <= map.getSourceLine().size() && "Invalid range!", file C:\Buildbot\Slave\llvm-clang-lld-x86_64-scei-ps4-windows10pro-fast\llvm.src\tools\clang\lib\Frontend\TextDiagnostic.cpp, line 999

clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp

public:
   unsigned getWarningsAsErrorsCount() const { return WarningsAsErrors; }
 
 private:
   SourceLocation getLocation(StringRef FilePath, unsigned Offset) {
     if (FilePath.empty())
       return SourceLocation();
 
     const FileEntry *File = SourceMgr.getFileManager().getFile(FilePath);
-    FileID ID = SourceMgr.createFileID(File, SourceLocation(), SrcMgr::C_User);
+    FileID ID = SourceMgr.getOrCreateFileID(File, SrcMgr::C_User);
     return SourceMgr.getLocForStartOfFile(ID).getLocWithOffset(Offset);
   }
 
   void reportNote(const tooling::DiagnosticMessage &Message) {
     SourceLocation Loc = getLocation(Message.FilePath, Message.FileOffset);
     Diags.Report(Loc, Diags.getCustomDiagID(DiagnosticsEngine::Note, "%0"))
         << Message.Message;
   }