[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
This revision was automatically updated to reflect the committed changes. Closed by commit rG2eff1c3ce48e: [clang-format] Extend AllowShortLoopsOnASingleLine to do ... while loops. (authored by mitchell-stellar). Changed prior to commit: https://reviews.llvm.org/D75022?vs=246512=248748#toc Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 Files: clang/lib/Format/UnwrappedLineFormatter.cpp clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTest.cpp === --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -556,6 +556,30 @@ verifyFormat("for (;;) /* still don't merge */\n" " continue;", AllowsMergedLoops); + verifyFormat("do a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do /* Don't merge */\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do\n" + " // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + // Without braces labels are interpreted differently. + verifyFormat("{\n" + " do\n" + " label:\n" + "a++;\n" + " while (true);\n" + "}", + AllowsMergedLoops); } TEST_F(FormatTest, FormatShortBracedStatements) { Index: clang/lib/Format/UnwrappedLineFormatter.cpp === --- clang/lib/Format/UnwrappedLineFormatter.cpp +++ clang/lib/Format/UnwrappedLineFormatter.cpp @@ -411,7 +411,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -514,7 +514,10 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) + return 0; +// Only merge do while if do is the only statement on the line. +if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; Index: clang/unittests/Format/FormatTest.cpp === --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -556,6 +556,30 @@ verifyFormat("for (;;) /* still don't merge */\n" " continue;", AllowsMergedLoops); + verifyFormat("do a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do /* Don't merge */\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do\n" + " // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + // Without braces labels are interpreted differently. + verifyFormat("{\n" + " do\n" + " label:\n" + "a++;\n" + " while (true);\n" + "}", + AllowsMergedLoops); } TEST_F(FormatTest, FormatShortBracedStatements) { Index: clang/lib/Format/UnwrappedLineFormatter.cpp === --- clang/lib/Format/UnwrappedLineFormatter.cpp +++ clang/lib/Format/UnwrappedLineFormatter.cpp @@ -411,7 +411,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -514,7 +514,10 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) + return 0; +// Only merge do while if do is the only statement on the line. +if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do)) return 0; if (1 + I[1]->Last->TotalLength >
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
MyDeveloperDay accepted this revision. MyDeveloperDay added a comment. This revision is now accepted and ready to land. LGTM, thank you for adding the extra test, please mark the inline comments as done CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
mitchell-stellar added a comment. Not that I am aware of. Whoever ends up doing the merge will likely run the necessary tests before committing. If you've run as many as you can, then hopefully all will be fine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
DaanDeMeyer added a comment. Is there CI infra that runs for each revision? I verified all the format unit tests still pass but I haven't run the entire test suite on my machine. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
mitchell-stellar accepted this revision. mitchell-stellar added a comment. Assuming this passes all existing tests, LGTM. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
DaanDeMeyer updated this revision to Diff 246512. DaanDeMeyer added a comment. Added extra unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 Files: lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -556,6 +556,30 @@ verifyFormat("for (;;) /* still don't merge */\n" " continue;", AllowsMergedLoops); + verifyFormat("do a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do /* Don't merge */\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do\n" + " // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + // Without braces labels are interpreted differently. + verifyFormat("{\n" + " do\n" + " label:\n" + "a++;\n" + " while (true);\n" + "}", + AllowsMergedLoops); } TEST_F(FormatTest, FormatShortBracedStatements) { Index: lib/Format/UnwrappedLineFormatter.cpp === --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -404,7 +404,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -449,7 +449,10 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) + return 0; +// Only merge do while if do is the only statement on the line. +if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -556,6 +556,30 @@ verifyFormat("for (;;) /* still don't merge */\n" " continue;", AllowsMergedLoops); + verifyFormat("do a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do /* Don't merge */\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do\n" + " // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + // Without braces labels are interpreted differently. + verifyFormat("{\n" + " do\n" + " label:\n" + "a++;\n" + " while (true);\n" + "}", + AllowsMergedLoops); } TEST_F(FormatTest, FormatShortBracedStatements) { Index: lib/Format/UnwrappedLineFormatter.cpp === --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -404,7 +404,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -449,7 +449,10 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) + return 0; +// Only merge do while if do is the only statement on the line. +if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
MyDeveloperDay added a comment. I think its look good, please add the extra tests, then lets give people a couple of days Comment at: unittests/Format/FormatTest.cpp:570 + AllowsMergedLoops); } horrible code though they are, could you add a test for the label and comment case ``` do label: a++ white(true); ``` and ``` do // comment a++ white(true); ``` CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
DaanDeMeyer updated this revision to Diff 246244. DaanDeMeyer added a comment. Added unit tests and fixed the case where stuff follows the do statement (like a comment). CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 Files: lib/Format/UnwrappedLineFormatter.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -556,6 +556,17 @@ verifyFormat("for (;;) /* still don't merge */\n" " continue;", AllowsMergedLoops); + verifyFormat("do a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do /* Don't merge */\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); } TEST_F(FormatTest, FormatShortBracedStatements) { Index: lib/Format/UnwrappedLineFormatter.cpp === --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -404,7 +404,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -449,7 +449,10 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) + return 0; +// Only merge do while if do is the only statement on the line. +if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -556,6 +556,17 @@ verifyFormat("for (;;) /* still don't merge */\n" " continue;", AllowsMergedLoops); + verifyFormat("do a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do /* Don't merge */\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); + verifyFormat("do // Don't merge\n" + " a++;\n" + "while (true);", + AllowsMergedLoops); } TEST_F(FormatTest, FormatShortBracedStatements) { Index: lib/Format/UnwrappedLineFormatter.cpp === --- lib/Format/UnwrappedLineFormatter.cpp +++ lib/Format/UnwrappedLineFormatter.cpp @@ -404,7 +404,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -449,7 +449,10 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) + return 0; +// Only merge do while if do is the only statement on the line. +if (Line.First->is(tok::kw_do) && !Line.Last->is(tok::kw_do)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
MyDeveloperDay added a comment. You need to add unit tests CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D75022: clang-format: Extend AllowShortLoopsOnASingleLine to do ... while loops.
DaanDeMeyer updated this revision to Diff 246150. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75022/new/ https://reviews.llvm.org/D75022 Files: clang/lib/Format/UnwrappedLineFormatter.cpp Index: clang/lib/Format/UnwrappedLineFormatter.cpp === --- clang/lib/Format/UnwrappedLineFormatter.cpp +++ clang/lib/Format/UnwrappedLineFormatter.cpp @@ -404,7 +404,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -449,7 +449,7 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; Index: clang/lib/Format/UnwrappedLineFormatter.cpp === --- clang/lib/Format/UnwrappedLineFormatter.cpp +++ clang/lib/Format/UnwrappedLineFormatter.cpp @@ -404,7 +404,7 @@ ? tryMergeSimpleControlStatement(I, E, Limit) : 0; } -if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while)) { +if (TheLine->First->isOneOf(tok::kw_for, tok::kw_while, tok::kw_do)) { return Style.AllowShortLoopsOnASingleLine ? tryMergeSimpleControlStatement(I, E, Limit) : 0; @@ -449,7 +449,7 @@ return 0; Limit = limitConsideringMacros(I + 1, E, Limit); AnnotatedLine = **I; -if (Line.Last->isNot(tok::r_paren)) +if (!Line.First->is(tok::kw_do) && Line.Last->isNot(tok::r_paren)) return 0; if (1 + I[1]->Last->TotalLength > Limit) return 0; ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits