[PATCH] D150848: [clang-format] Respect ColumnLimit 0 lines breaks in inline asm
rymiel updated this revision to Diff 530486. rymiel added a comment. Apply suggestions from review Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150848/new/ https://reviews.llvm.org/D150848 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTest.cpp === --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -4617,6 +4617,24 @@ format("__asm {\n" "}\n" "int i;")); + + auto Style = getLLVMStyleWithColumns(0); + const StringRef Code1{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));"}; + const StringRef Code2{"asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));"}; + const StringRef Code3{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));"}; + + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; + verifyFormat(Code1, Style); + EXPECT_EQ(Code2, format(Code2, Style)); + EXPECT_EQ(Code3, format(Code3, Style)); + + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always; + EXPECT_EQ(Code2, format(Code1, Style)); + EXPECT_EQ(Code2, format(Code2, Style)); + EXPECT_EQ(Code2, format(Code3, Style)); } TEST_F(FormatTest, FormatTryCatch) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -357,7 +357,8 @@ if (Current.MustBreakBefore || (Current.is(TT_InlineASMColon) && (Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always || -Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) { +(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline && + Style.ColumnLimit > 0 { return true; } if (CurrentState.BreakBeforeClosingBrace && Index: clang/unittests/Format/FormatTest.cpp === --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -4617,6 +4617,24 @@ format("__asm {\n" "}\n" "int i;")); + + auto Style = getLLVMStyleWithColumns(0); + const StringRef Code1{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));"}; + const StringRef Code2{"asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));"}; + const StringRef Code3{"asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));"}; + + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; + verifyFormat(Code1, Style); + EXPECT_EQ(Code2, format(Code2, Style)); + EXPECT_EQ(Code3, format(Code3, Style)); + + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always; + EXPECT_EQ(Code2, format(Code1, Style)); + EXPECT_EQ(Code2, format(Code2, Style)); + EXPECT_EQ(Code2, format(Code3, Style)); } TEST_F(FormatTest, FormatTryCatch) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -357,7 +357,8 @@ if (Current.MustBreakBefore || (Current.is(TT_InlineASMColon) && (Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always || -Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) { +(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline && + Style.ColumnLimit > 0 { return true; } if (CurrentState.BreakBeforeClosingBrace && ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D150848: [clang-format] Respect ColumnLimit 0 lines breaks in inline asm
owenpan accepted this revision. owenpan added inline comments. Comment at: clang/lib/Format/ContinuationIndenter.cpp:361 +(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline && + Style.ColumnLimit != 0 { return true; FWIW. Comment at: clang/unittests/Format/FormatTest.cpp:4622-4655 + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; + verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style); + EXPECT_EQ("asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" FWIW, it's easier to read IMO. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D150848/new/ https://reviews.llvm.org/D150848 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D150848: [clang-format] Respect ColumnLimit 0 lines breaks in inline asm
rymiel created this revision. rymiel added a project: clang-format. rymiel added reviewers: HazardyKnusperkeks, owenpan, MyDeveloperDay. Herald added projects: All, clang. Herald added a subscriber: cfe-commits. rymiel requested review of this revision. Previously, using ColumnLimit: 0 with extended inline asm with the BreakBeforeInlineASMColon: OnlyMultiline option (the default style), the formatter would act as if in Always mode, meaning a line break was added before every colon in an extended inline assembly block. This patch respects the already existing line breaks, and doesn't add any new ones, if in ColumnLimit 0 mode. Behaviour with Always stays as expected, with a break before every colon regardless of any existing line breaks. Behaviour with Never was broken before, and remains broken with this patch, it is just never respected in ColumnLimit 0 mode. Fixes https://github.com/llvm/llvm-project/issues/62754 Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D150848 Files: clang/lib/Format/ContinuationIndenter.cpp clang/unittests/Format/FormatTest.cpp Index: clang/unittests/Format/FormatTest.cpp === --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -4617,6 +4617,42 @@ format("__asm {\n" "}\n" "int i;")); + + auto Style = getLLVMStyleWithColumns(0); + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; + verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style); + EXPECT_EQ("asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + Style)); + EXPECT_EQ("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + Style)); + + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always; + EXPECT_EQ( + "asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style)); + EXPECT_EQ("asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + Style)); + EXPECT_EQ("asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + Style)); } TEST_F(FormatTest, FormatTryCatch) { Index: clang/lib/Format/ContinuationIndenter.cpp === --- clang/lib/Format/ContinuationIndenter.cpp +++ clang/lib/Format/ContinuationIndenter.cpp @@ -357,7 +357,8 @@ if (Current.MustBreakBefore || (Current.is(TT_InlineASMColon) && (Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_Always || -Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline))) { +(Style.BreakBeforeInlineASMColon == FormatStyle::BBIAS_OnlyMultiline && + Style.ColumnLimit != 0 { return true; } if (CurrentState.BreakBeforeClosingBrace && Index: clang/unittests/Format/FormatTest.cpp === --- clang/unittests/Format/FormatTest.cpp +++ clang/unittests/Format/FormatTest.cpp @@ -4617,6 +4617,42 @@ format("__asm {\n" "}\n" "int i;")); + + auto Style = getLLVMStyleWithColumns(0); + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_OnlyMultiline; + verifyFormat("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style); + EXPECT_EQ("asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + Style)); + EXPECT_EQ("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + Style)); + + Style.BreakBeforeInlineASMColon = FormatStyle::BBIAS_Always; + EXPECT_EQ( + "asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" + ": \"a\"(data));", + format("asm(\"xyz\" : \"=a\"(a), \"=d\"(b) : \"a\"(data));", Style)); + EXPECT_EQ("asm(\"xyz\"\n" +": \"=a\"(a), \"=d\"(b)\n" +": \"a\"(data));", +format("asm(\"xyz\"\n" + ": \"=a\"(a), \"=d\"(b)\n" +