HazardyKnusperkeks requested changes to this revision.
HazardyKnusperkeks added inline comments.
This revision now requires changes to proceed.
================
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"
----------------
But this line is longer than 20! This test should fail, shouldn't it?
================
Comment at: clang/unittests/Format/FormatTest.cpp:16352
+template <size_t M, size_t N>
+auto createStylesImpl(
----------------
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.
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
