Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-14 Thread Samuel Benzaquen via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL266369: [clang-tidy] Add check misc-multiple-statement-macro (authored by sbenza). Changed prior to commit: http://reviews.llvm.org/D18766?vs=53398&id=53786#toc Repository: rL LLVM http://reviews.ll

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-12 Thread Alexander Kornienko via cfe-commits
alexfh added inline comments. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:100 @@ +99,3 @@ + diag(InnerRanges.back().first, "multiple statement macro used without " + "braces; some statements will be " +

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-12 Thread Samuel Benzaquen via cfe-commits
sbenza added inline comments. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:99 @@ +98,3 @@ + + diag(InnerRanges.back().first, "multiple statement macro spans unbraced " + "conditional and the following statement"); al

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-12 Thread Samuel Benzaquen via cfe-commits
sbenza updated this revision to Diff 53398. sbenza marked an inline comment as done. sbenza added a comment. Change warning message. http://reviews.llvm.org/D18766 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MultipleStatementMacroCheck.cpp cl

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-07 Thread Alexander Kornienko via cfe-commits
alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land. LG with a nit. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:99 @@ +98,3 @@ + + diag(InnerRanges.back().first, "multiple statement macro spans unbraced " +

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-05 Thread Samuel Benzaquen via cfe-commits
sbenza added inline comments. Comment at: docs/clang-tidy/checks/misc-multiple-statement-macro.rst:15 @@ +14,3 @@ + if (do_increment) +INCREMENT_TWO(a, b); + hokein wrote: > Would be better to add a comment to explain the sample. The sentence just before the

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-05 Thread Samuel Benzaquen via cfe-commits
sbenza updated this revision to Diff 52702. sbenza marked 2 inline comments as done and an inline comment as not done. sbenza added a comment. Minor fixes http://reviews.llvm.org/D18766 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MultipleStatem

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-05 Thread Haojian Wu via cfe-commits
hokein added a comment. Nice check! I like the check, it would be very helpful. (I have struggled with this kind of bug before) Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:33 @@ +32,3 @@ + +namespace { + I think you can put this anonymous namesp

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-04 Thread Etienne Bergeron via cfe-commits
etienneb added a comment. thanks, lgtm. http://reviews.llvm.org/D18766 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-04 Thread Samuel Benzaquen via cfe-commits
sbenza marked an inline comment as done and an inline comment as not done. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:33 @@ +32,3 @@ + +namespace { + etienneb wrote: > I feel it nicer if you merge this namespace with the one at line 20. I like to

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-04 Thread Samuel Benzaquen via cfe-commits
sbenza updated this revision to Diff 52625. sbenza marked an inline comment as done. sbenza added a comment. Minor fixes http://reviews.llvm.org/D18766 Files: clang-tidy/misc/CMakeLists.txt clang-tidy/misc/MiscTidyModule.cpp clang-tidy/misc/MultipleStatementMacroCheck.cpp clang-tidy/mis

Re: [PATCH] D18766: [clang-tidy] Add check misc-multiple-statement-macro

2016-04-04 Thread Etienne Bergeron via cfe-commits
etienneb added a subscriber: etienneb. etienneb added a comment. this is cool. Comment at: clang-tidy/misc/MultipleStatementMacroCheck.cpp:33 @@ +32,3 @@ + +namespace { + I feel it nicer if you merge this namespace with the one at line 20. Comm