Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein added a comment. @chapuni, thanks! Repository: rL LLVM http://reviews.llvm.org/D19482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

chapuni added a subscriber: chapuni. Comment at: clang-tools-extra/trunk/unittests/include-fixer/find-all-symbols/FindAllSymbolsTests.cpp:333 @@ +332,3 @@ + static const char Code[] = R"( + typedef unsigned size_t; + typedef struct { int x; } X; size_t

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

This revision was automatically updated to reflect the committed changes. Closed by commit rL267719: [include-fixer] Add a find-all-symbols tool for include-fixer. (authored by hokein). Changed prior to commit: http://reviews.llvm.org/D19482?vs=55226&id=55227#toc Repository: rL LLVM http://

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein updated this revision to Diff 55226. hokein added a comment. Fix another nit. http://reviews.llvm.org/D19482 Files: include-fixer/CMakeLists.txt include-fixer/find-all-symbols/CMakeLists.txt include-fixer/find-all-symbols/FindAllSymbols.cpp include-fixer/find-all-symbols/FindAllS

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein updated this revision to Diff 55223. hokein added a comment. Fix a nit. http://reviews.llvm.org/D19482 Files: include-fixer/CMakeLists.txt include-fixer/find-all-symbols/CMakeLists.txt include-fixer/find-all-symbols/FindAllSymbols.cpp include-fixer/find-all-symbols/FindAllSymbols

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

djasper accepted this revision. djasper added a comment. Looks good. Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:39 @@ +38,3 @@ +} else { + const auto *RD = llvm::cast(Context); + Symbol->Contexts.emplace_back(SymbolInfo::Record, RD->getName().str

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein added inline comments. Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:22-24 @@ +21,5 @@ + +using SymbolInfo = clang::find_all_symbols::SymbolInfo; +using SymbolType = clang::find_all_symbols::SymbolInfo::SymbolType; +using ContextType = clang::find_all_symbol

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein updated this revision to Diff 55220. hokein marked 13 inline comments as done. hokein added a comment. Add Daniel's comments. http://reviews.llvm.org/D19482 Files: include-fixer/CMakeLists.txt include-fixer/find-all-symbols/CMakeLists.txt include-fixer/find-all-symbols/FindAllSymbo

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

djasper added inline comments. Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:22-24 @@ +21,5 @@ + +using SymbolInfo = clang::find_all_symbols::SymbolInfo; +using SymbolType = clang::find_all_symbols::SymbolInfo::SymbolType; +using ContextType = clang::find_all_symbo

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

bkramer added inline comments. Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:39 @@ +38,3 @@ + ResultReporter *const Reporter; +}; + Ah, sorry, my bad. http://reviews.llvm.org/D19482 ___ cfe-commits ma

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein added inline comments. Comment at: include-fixer/find-all-symbols/FindAllSymbols.h:38 @@ +37,3 @@ +private: + ResultReporter *const Reporter; +}; bkramer wrote: > nit: move const to the start of the line. Moving `const` to the start of line will change se

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein updated this revision to Diff 55172. hokein marked 2 inline comments as done. hokein added a comment. Update. http://reviews.llvm.org/D19482 Files: include-fixer/CMakeLists.txt include-fixer/find-all-symbols/CMakeLists.txt include-fixer/find-all-symbols/FindAllSymbols.cpp include

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

bkramer accepted this revision. bkramer added a comment. This revision is now accepted and ready to land. lg with some minor comments remaining. Comment at: include-fixer/find-all-symbols/FindAllSymbols.cpp:47 @@ +46,3 @@ + const auto *RD = llvm::dyn_cast(Context); + a

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein marked an inline comment as done. hokein added a comment. http://reviews.llvm.org/D19482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

hokein updated this revision to Diff 55003. hokein marked 3 inline comments as done. hokein added a comment. Address review comments. http://reviews.llvm.org/D19482 Files: include-fixer/CMakeLists.txt include-fixer/find-all-symbols/CMakeLists.txt include-fixer/find-all-symbols/FindAllSymb

Re: [PATCH] D19482: [include-fixer] Add a find-all-symbols tool for include-fixer.

bkramer added a comment. First, please run this through clang-format with LLVM style. This aligns all the & and * to the name instead of the type. There are also some underscore_names left that need conversion to PascalCase. Comment at: include-fixer/find-all-symbols/FindAllSy