[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-02-28 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. In https://reviews.llvm.org/D42729#1022069, @Typz wrote: > In https://reviews.llvm.org/D42729#994841, @djasper wrote: > > > - Of course you find all sorts of errors while testing clang-format on a > > large-enough codebase. That doesn't mean that users run into them much

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-02-28 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42729#994841, @djasper wrote: > - Of course you find all sorts of errors while testing clang-format on a > large-enough codebase. That doesn't mean that users run into them much. > - We have had about 10k clang-format users internally for severa

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-02-01 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. - Of course you find all sorts of errors while testing clang-format on a large-enough codebase. That doesn't mean that users run into them much. - We have had about 10k clang-format users internally for several years. The semicolon issue comes up but really rarely and if

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-02-01 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. > I don't think cases where there is only a semicolon-less macro are > particularly frequent/important either. You probably could even add a > semicolon in those cases, right? How frequent are such cases in your > codebase? Either way, they aren't fixed by this patch, so t

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. I don't think cases where there is only a semicolon-less macro are particularly frequent/important either. You probably could even add a semicolon in those cases, right? How frequent are such cases in your codebase? Either way, they aren't fixed by this patch, so they a

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. In https://reviews.llvm.org/D42729#993159, @djasper wrote: > I think this case is not important enough to fix. Please tell users to just > delete the useless semicolon. I would agree if were simple to spot: but often this may manifest itself only with a missing space bet

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added a comment. There are actually other cases, e.g. with macros "containing" the semicolon: void abort() { FOO() BAR() }; gets reformatted to this (still wrong with this patch, but the space after the parenthesis is added): void abort(){ FOO() BAR() }; And also this one (

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Daniel Jasper via Phabricator via cfe-commits
djasper added a comment. I think this case is not important enough to fix. Please tell users to just delete the useless semicolon. In particular, my concern is that this makes the data-flow significantly more complicated. Early on in the development, we had separate token classes for the diffe

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz added inline comments. Comment at: lib/Format/TokenAnnotator.cpp:1911 + if (Next && Next->is(tok::l_brace) && Next->BlockKind == BK_BracedInit) + Next->BlockKind = BK_Block; +} this may actually not be enough in all cases: to completely mat

[PATCH] D42729: clang-format: Fix formatting of function body followed by semicolon

2018-01-31 Thread Francois Ferrand via Phabricator via cfe-commits
Typz updated this revision to Diff 132130. Typz added a comment. update commit message Repository: rC Clang https://reviews.llvm.org/D42729 Files: lib/Format/TokenAnnotator.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp ===