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

2017-04-06 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299700: [clang-tidy] Reuse FileID in getLocation (authored 
by chh).

Repository:
  rL LLVM

https://reviews.llvm.org/D31406

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


Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -239,7 +239,7 @@
   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);
   }
 


Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -239,7 +239,7 @@
   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);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-03-31 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
chh reopened this revision.
chh added a comment.
This revision is now accepted and ready to land.

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


Repository:
  rL LLVM

https://reviews.llvm.org/D31406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-03-30 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
This revision was automatically updated to reflect the committed changes.
Closed by commit rL299119: [clang-tidy] Reuse FileID in getLocation (authored 
by chh).

Changed prior to commit:
  https://reviews.llvm.org/D31406?vs=93499=93554#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D31406

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


Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -238,7 +238,7 @@
   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);
   }
 


Index: clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
===
--- clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
+++ clang-tools-extra/trunk/clang-tidy/ClangTidy.cpp
@@ -238,7 +238,7 @@
   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);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-03-30 Thread Alexander Kornienko via Phabricator via cfe-commits 
alexfh accepted this revision.
alexfh added a comment.
This revision is now accepted and ready to land.

LG
Do you have commit rights?


https://reviews.llvm.org/D31406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-03-30 Thread Chih-Hung Hsieh via Phabricator via cfe-commits 
chh updated this revision to Diff 93499.
chh marked an inline comment as done.
chh added a comment.

Use getOrCreateFileID.


https://reviews.llvm.org/D31406

Files:
  clang-tidy/ClangTidy.cpp


Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -238,7 +238,7 @@
   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);
   }
 


Index: clang-tidy/ClangTidy.cpp
===
--- clang-tidy/ClangTidy.cpp
+++ clang-tidy/ClangTidy.cpp
@@ -238,7 +238,7 @@
   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);
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

2017-03-30 Thread 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  = 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 Path2FileID;
   FileManager Files;

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


https://reviews.llvm.org/D31406



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits