hokein updated this revision to Diff 57493.
hokein added a comment.
Rebase.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbols/FindAllSymbols.h
include-fixer/find-all-symbol
This revision was automatically updated to reflect the committed changes.
Closed by commit rL269779: [find-all-symbols] Add IWYU private pragma support.
(authored by hokein).
Changed prior to commit:
http://reviews.llvm.org/D19816?vs=57493&id=57495#toc
Repository:
rL LLVM
http://reviews.llv
hokein updated this revision to Diff 57490.
hokein added a comment.
Add comments.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbols/FindAllSymbols.h
include-fixer/find-all-
hokein marked an inline comment as done.
hokein added a comment.
http://reviews.llvm.org/D19816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
klimek accepted this revision.
klimek added a comment.
This revision is now accepted and ready to land.
LG with nit to fix.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:44-45
@@ -41,2 +43,4 @@
- explicit FindAllSymbols(ResultReporter *Reporter) : Reporter(Repor
hokein updated this revision to Diff 57472.
hokein added a comment.
Rebase.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbols/FindAllSymbols.h
include-fixer/find-all-symbol
hokein marked an inline comment as done.
hokein added a comment.
http://reviews.llvm.org/D19816
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
hokein updated this revision to Diff 56872.
hokein marked an inline comment as done.
hokein added a comment.
Rebase to master.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbo
hokein marked an inline comment as done.
Comment at: include-fixer/find-all-symbols/PragmaCommentHandler.cpp:27
@@ +26,3 @@
+ SmallVector Matches;
+ if (!llvm::Regex(IWYUPragma).match(Text, &Matches))
+return false;
bkramer wrote:
> Isn't this regex just a c
hokein updated this revision to Diff 56579.
hokein marked an inline comment as done.
hokein added a comment.
Address comments.
http://reviews.llvm.org/D19816
Files:
clang-tidy/misc/UnusedUsingDeclsCheck.cpp
clang-tidy/misc/UnusedUsingDeclsCheck.h
include-fixer/find-all-symbols/CMakeLists.
bkramer added inline comments.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:16
@@ +15,3 @@
+#include "llvm/ADT/StringRef.h"
+
+#include
No space between #includes in LLVM.
Comment at: include-fixer/find-all-symbols/PragmaCommentHa
klimek added inline comments.
Comment at: unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp:418
@@ +417,3 @@
+
+ {
+SymbolInfo Symbol =
Why are the other test cases using an extra block?
http://reviews.llvm.org/D19816
_
hokein added inline comments.
Comment at: unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp:418
@@ +417,3 @@
+
+ {
+SymbolInfo Symbol =
klimek wrote:
> Any reason for the extra block here?
Because I want to keep consistence with other test case
hokein updated this revision to Diff 56130.
hokein marked an inline comment as done.
hokein added a comment.
Update.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbols/FindAll
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:48
@@ +47,3 @@
+ explicit FindAllSymbols(ResultReporter *Reporter,
+ HeaderMapCollector *Collector)
+ : Reporter(Reporter), Collector(Collector) {}
hokein updated this revision to Diff 56121.
hokein added a comment.
Fix a nit.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbols/FindAllSymbols.h
include-fixer/find-all-sym
hokein updated this revision to Diff 56120.
hokein added a comment.
Improvements based on offline discussion.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-all-symbols/FindAllSymbols
hokein updated this revision to Diff 56101.
hokein marked 2 inline comments as done.
hokein added a comment.
Address review comments.
http://reviews.llvm.org/D19816
Files:
include-fixer/find-all-symbols/CMakeLists.txt
include-fixer/find-all-symbols/FindAllSymbols.cpp
include-fixer/find-al
Re semantics, you may want to link to IWYU's docs at
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md
.
- Kim
Den 3 maj 2016 6:34 em skrev "Manuel Klimek via cfe-commits" <
cfe-commits@lists.llvm.org>:
> klimek added inline comments.
>
> ===
kimgr added a subscriber: kimgr.
kimgr added a comment.
Re semantics, you may want to link to IWYU's docs at
https://github.com/include-what-you-use/include-what-you-use/blob/master/docs/IWYUPragmas.md
.
- Kim
Den 3 maj 2016 6:34 em skrev "Manuel Klimek via cfe-commits" <
cfe-commits@lists.llvm.
klimek added inline comments.
Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:173-182
@@ -165,1 +172,12 @@
+void FindAllSymbols::addPragmaHeader(FileID ID, llvm::StringRef FilePath) {
+ PragmaHeaderMap[ID] = FilePath;
+}
+
+llvm::Optional FindAllSymbols::getPragma
21 matches
Mail list logo
