feg208 added a comment.
I still need to handle the 20 character case appropriately
================
Comment at: clang/unittests/Format/FormatTest.cpp:16465
+ "test demo[] = {\n"
+ " {56, 23, \"hello world i am a very long line that really, in any
"
+ "just world, ought to be split over multiple lines\" },\n"
----------------
HazardyKnusperkeks wrote:
> But this line is longer than 20! This test should fail, shouldn't it?
It should. When I use the command line invocation I get different output
```
#include <array>
struct test {
int a, b;
const char *c;
};
int main(
int,
const char **) {
std::array<
struct test,
3>
demo;
demo = std::array<
struct test,
3>{
test{
56, 23,
"hello "
"world i "
"am a "
"very "
"long "
"line "
"that "
"really, "
"in any "
"just "
"world, "
"ought "
"to be "
"split "
"over "
"multiple"
" lines"},
test{-1,
93463,
"world"},
test{7, 5,
"!!"},
};
}
```
I will investigate
================
Comment at: clang/unittests/Format/FormatTest.cpp:16352
+template <size_t M, size_t N>
+auto createStylesImpl(
----------------
HazardyKnusperkeks wrote:
> feg208 wrote:
> > curdeius wrote:
> > > HazardyKnusperkeks wrote:
> > > > This is a nice approach!
> > > >
> > > > Could it be made constexpr? I'm not sure about C++14.
> > > Personally, I'm not really fond of testing all the combinations of
> > > styles. It adds little value to those tests.
> > > I find it interesting to find problematic edge cases (which then should
> > > be added to the test suite "manually").
> > > Also, when a test like this fails, it would be hard to see with which
> > > style it failed.
> > I think it could be constexpr (thats the direction I originally started
> > which lead to much moaning about the lack of fold expressions) however
> > getLLVMStyle() is not constexpr and it felt inappropriate for this commit
> > to change the interface to that function even if the change were relatively
> > harmless.
> > Also, when a test like this fails, it would be hard to see with which style
> > it failed.
>
> That is a valid point. You should only test some combinations.
I can certainly strip it down to corner cases. I guess I am not clear on which
ones I should focus on. When I implemented this none of the combinations
created problems. I could pick a few at random? But that doesn't feel like an
improvement.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D101868/new/
https://reviews.llvm.org/D101868
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
