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
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
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
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
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
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
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 (
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
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
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
===
10 matches
Mail list logo