[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
This revision was automatically updated to reflect the committed changes. Closed by commit rL310539: clang-format: Fix bug with ENAS_DontAlign and empty lines (authored by jtbandes). Repository: rL LLVM https://reviews.llvm.org/D36019 Files: cfe/trunk/lib/Format/WhitespaceManager.cpp cfe/trunk/lib/Format/WhitespaceManager.h cfe/trunk/unittests/Format/FormatTest.cpp Index: cfe/trunk/lib/Format/WhitespaceManager.h === --- cfe/trunk/lib/Format/WhitespaceManager.h +++ cfe/trunk/lib/Format/WhitespaceManager.h @@ -195,9 +195,9 @@ /// \brief Stores \p Text as the replacement for the whitespace in \p Range. void storeReplacement(SourceRange Range, StringRef Text); void appendNewlineText(std::string , unsigned Newlines); - void appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn); + void appendEscapedNewlineText(std::string , unsigned Newlines, +unsigned PreviousEndOfTokenColumn, +unsigned EscapedNewlineColumn); void appendIndentText(std::string , unsigned IndentLevel, unsigned Spaces, unsigned WhitespaceStartColumn); Index: cfe/trunk/lib/Format/WhitespaceManager.cpp === --- cfe/trunk/lib/Format/WhitespaceManager.cpp +++ cfe/trunk/lib/Format/WhitespaceManager.cpp @@ -603,8 +603,9 @@ if (C.CreateReplacement) { std::string ReplacementText = C.PreviousLinePostfix; if (C.ContinuesPPDirective) -appendNewlineText(ReplacementText, C.NewlinesBefore, - C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn); +appendEscapedNewlineText(ReplacementText, C.NewlinesBefore, + C.PreviousEndOfTokenColumn, + C.EscapedNewlineColumn); else appendNewlineText(ReplacementText, C.NewlinesBefore); appendIndentText(ReplacementText, C.Tok->IndentLevel, @@ -640,16 +641,17 @@ Text.append(UseCRLF ? "\r\n" : "\n"); } -void WhitespaceManager::appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn) { +void WhitespaceManager::appendEscapedNewlineText(std::string , + unsigned Newlines, + unsigned PreviousEndOfTokenColumn, + unsigned EscapedNewlineColumn) { if (Newlines > 0) { -unsigned Offset = -std::min(EscapedNewlineColumn - 2, PreviousEndOfTokenColumn); +unsigned Spaces = +std::max(1, EscapedNewlineColumn - PreviousEndOfTokenColumn - 1); for (unsigned i = 0; i < Newlines; ++i) { - Text.append(EscapedNewlineColumn - Offset - 1, ' '); + Text.append(Spaces, ' '); Text.append(UseCRLF ? "\\\r\n" : "\\\n"); - Offset = 0; + Spaces = std::max(0, EscapedNewlineColumn - 1); } } } Index: cfe/trunk/unittests/Format/FormatTest.cpp === --- cfe/trunk/unittests/Format/FormatTest.cpp +++ cfe/trunk/unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time it's reformatted + EXPECT_EQ("#define A \\\n" +" class Foo { \\\n" +"void bar(); \\\n" +"\\\n" +"\\\n" +"\\\n" +" public: \\\n" +"void baz(); \\\n" +" };", +format("#define A \\\n" + " class Foo { \\\n" + "void bar(); \\\n" + "\\\n" + "\\\n" + "\\\n" + " public: \\\n" + "void baz(); \\\n" + " };", DontAlign)); } TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) { Index: cfe/trunk/lib/Format/WhitespaceManager.h === --- cfe/trunk/lib/Format/WhitespaceManager.h +++ cfe/trunk/lib/Format/WhitespaceManager.h @@ -195,9 +195,9 @@ /// \brief Stores \p Text as the replacement for the whitespace in \p Range. void storeReplacement(SourceRange Range, StringRef Text); void appendNewlineText(std::string , unsigned
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes added a comment. Thanks. Can you commit this when you get a chance? I don't have permissions. https://reviews.llvm.org/D36019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
djasper accepted this revision. djasper added a comment. This revision is now accepted and ready to land. Thanks you. Comment at: lib/Format/WhitespaceManager.cpp:650 +for (unsigned i = 0; i < Newlines; ++i) + Text.append(UseCRLF ? " \\\r\n" : " \\\n"); +return; jtbandes wrote: > djasper wrote: > > Note that when you have an empty line, this would turn into: > > > > #define A \ > > int i; \ > >\<-- Note the 1-space indent here. > > int j; \ > > int k; > > > > With my alternative below, that "\" will just be put at column 0, which > > probably isn't better or worse. > I suppose that can be changed back to 1 by using `std::max(1, > EscapedNewlineColumn - 1);` instead, right? I don't have strong feelings > about whether it should be 0 or 1. I don't either. Leave as is for now. https://reviews.llvm.org/D36019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:650 +for (unsigned i = 0; i < Newlines; ++i) + Text.append(UseCRLF ? " \\\r\n" : " \\\n"); +return; djasper wrote: > Note that when you have an empty line, this would turn into: > > #define A \ > int i; \ >\<-- Note the 1-space indent here. > int j; \ > int k; > > With my alternative below, that "\" will just be put at column 0, which > probably isn't better or worse. I suppose that can be changed back to 1 by using `std::max(1, EscapedNewlineColumn - 1);` instead, right? I don't have strong feelings about whether it should be 0 or 1. https://reviews.llvm.org/D36019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes updated this revision to Diff 109898. jtbandes added a comment. @djasper ok, done https://reviews.llvm.org/D36019 Files: lib/Format/WhitespaceManager.cpp lib/Format/WhitespaceManager.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time it's reformatted + EXPECT_EQ("#define A \\\n" +" class Foo { \\\n" +"void bar(); \\\n" +"\\\n" +"\\\n" +"\\\n" +" public: \\\n" +"void baz(); \\\n" +" };", +format("#define A \\\n" + " class Foo { \\\n" + "void bar(); \\\n" + "\\\n" + "\\\n" + "\\\n" + " public: \\\n" + "void baz(); \\\n" + " };", DontAlign)); } TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) { Index: lib/Format/WhitespaceManager.h === --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -195,9 +195,9 @@ /// \brief Stores \p Text as the replacement for the whitespace in \p Range. void storeReplacement(SourceRange Range, StringRef Text); void appendNewlineText(std::string , unsigned Newlines); - void appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn); + void appendEscapedNewlineText(std::string , unsigned Newlines, +unsigned PreviousEndOfTokenColumn, +unsigned EscapedNewlineColumn); void appendIndentText(std::string , unsigned IndentLevel, unsigned Spaces, unsigned WhitespaceStartColumn); Index: lib/Format/WhitespaceManager.cpp === --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -603,8 +603,9 @@ if (C.CreateReplacement) { std::string ReplacementText = C.PreviousLinePostfix; if (C.ContinuesPPDirective) -appendNewlineText(ReplacementText, C.NewlinesBefore, - C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn); +appendEscapedNewlineText(ReplacementText, C.NewlinesBefore, + C.PreviousEndOfTokenColumn, + C.EscapedNewlineColumn); else appendNewlineText(ReplacementText, C.NewlinesBefore); appendIndentText(ReplacementText, C.Tok->IndentLevel, @@ -640,16 +641,17 @@ Text.append(UseCRLF ? "\r\n" : "\n"); } -void WhitespaceManager::appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn) { +void WhitespaceManager::appendEscapedNewlineText(std::string , + unsigned Newlines, + unsigned PreviousEndOfTokenColumn, + unsigned EscapedNewlineColumn) { if (Newlines > 0) { -unsigned Offset = -std::min(EscapedNewlineColumn - 2, PreviousEndOfTokenColumn); +unsigned Spaces = +std::max(1, EscapedNewlineColumn - PreviousEndOfTokenColumn - 1); for (unsigned i = 0; i < Newlines; ++i) { - Text.append(EscapedNewlineColumn - Offset - 1, ' '); + Text.append(Spaces, ' '); Text.append(UseCRLF ? "\\\r\n" : "\\\n"); - Offset = 0; + Spaces = std::max(0, EscapedNewlineColumn - 1); } } } Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
djasper added inline comments. Comment at: lib/Format/WhitespaceManager.cpp:650 +for (unsigned i = 0; i < Newlines; ++i) + Text.append(UseCRLF ? " \\\r\n" : " \\\n"); +return; Note that when you have an empty line, this would turn into: #define A \ int i; \ \<-- Note the 1-space indent here. int j; \ int k; With my alternative below, that "\" will just be put at column 0, which probably isn't better or worse. Comment at: lib/Format/WhitespaceManager.cpp:656 +assert(EscapedNewlineColumn >= 1); unsigned Offset = +std::min(EscapedNewlineColumn - 1, PreviousEndOfTokenColumn); You could change this to: unsigned Spaces = std::max(1, EscapedNewlineColumn - PreviousEndOfTokenColumn - 1); for (unsigned i = 0; i < Newlines; ++i) { Text.append(Spaces, ' '); Text.append(UseCRLF ? "\\\r\n" : "\\\n"); Spaces = std::max(0, EscapedNewlineColumn - 1); } And it should work without problems and without special code path. https://reviews.llvm.org/D36019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes added a comment. @djasper Bump :) https://reviews.llvm.org/D36019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes updated this revision to Diff 108863. jtbandes added a comment. - Undo change in argument list https://reviews.llvm.org/D36019 Files: lib/Format/WhitespaceManager.cpp lib/Format/WhitespaceManager.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time it's reformatted + EXPECT_EQ("#define A \\\n" +" class Foo { \\\n" +"void bar(); \\\n" +" \\\n" +" \\\n" +" \\\n" +" public: \\\n" +"void baz(); \\\n" +" };", +format("#define A \\\n" + " class Foo { \\\n" + "void bar(); \\\n" + " \\\n" + " \\\n" + " \\\n" + " public: \\\n" + "void baz(); \\\n" + " };", DontAlign)); } TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) { Index: lib/Format/WhitespaceManager.h === --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -195,9 +195,9 @@ /// \brief Stores \p Text as the replacement for the whitespace in \p Range. void storeReplacement(SourceRange Range, StringRef Text); void appendNewlineText(std::string , unsigned Newlines); - void appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn); + void appendEscapedNewlineText(std::string , unsigned Newlines, +unsigned PreviousEndOfTokenColumn, +unsigned EscapedNewlineColumn); void appendIndentText(std::string , unsigned IndentLevel, unsigned Spaces, unsigned WhitespaceStartColumn); Index: lib/Format/WhitespaceManager.cpp === --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -603,8 +603,9 @@ if (C.CreateReplacement) { std::string ReplacementText = C.PreviousLinePostfix; if (C.ContinuesPPDirective) -appendNewlineText(ReplacementText, C.NewlinesBefore, - C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn); +appendEscapedNewlineText(ReplacementText, C.NewlinesBefore, + C.PreviousEndOfTokenColumn, + C.EscapedNewlineColumn); else appendNewlineText(ReplacementText, C.NewlinesBefore); appendIndentText(ReplacementText, C.Tok->IndentLevel, @@ -640,12 +641,20 @@ Text.append(UseCRLF ? "\r\n" : "\n"); } -void WhitespaceManager::appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn) { +void WhitespaceManager::appendEscapedNewlineText(std::string , + unsigned Newlines, + unsigned PreviousEndOfTokenColumn, + unsigned EscapedNewlineColumn) { + if (Style.AlignEscapedNewlines == FormatStyle::ENAS_DontAlign) { +for (unsigned i = 0; i < Newlines; ++i) + Text.append(UseCRLF ? " \\\r\n" : " \\\n"); +return; + } + if (Newlines > 0) { +assert(EscapedNewlineColumn >= 1); unsigned Offset = -std::min(EscapedNewlineColumn - 2, PreviousEndOfTokenColumn); +std::min(EscapedNewlineColumn - 1, PreviousEndOfTokenColumn); for (unsigned i = 0; i < Newlines; ++i) { Text.append(EscapedNewlineColumn - Offset - 1, ' '); Text.append(UseCRLF ? "\\\r\n" : "\\\n"); Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes updated this revision to Diff 108860. jtbandes added a comment. Okay, I think this approach is better: - Rename the version of `appendNewlineText` used for escaped newlines to `appendEscapedNewlineText` to reduce confusability. - If `ENAS_DontAlign`, skip all of the offset calculation logic. Just append space-backslash-newline. - Restore the offset calculation to use `EscapedNewlineColumn - 1`, which it was before https://reviews.llvm.org/D32733. I don't think there was a good reason to change this. - Leave in the `assert`. https://reviews.llvm.org/D36019 Files: lib/Format/WhitespaceManager.cpp lib/Format/WhitespaceManager.h unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time it's reformatted + EXPECT_EQ("#define A \\\n" +" class Foo { \\\n" +"void bar(); \\\n" +" \\\n" +" \\\n" +" \\\n" +" public: \\\n" +"void baz(); \\\n" +" };", +format("#define A \\\n" + " class Foo { \\\n" + "void bar(); \\\n" + " \\\n" + " \\\n" + " \\\n" + " public: \\\n" + "void baz(); \\\n" + " };", DontAlign)); } TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) { Index: lib/Format/WhitespaceManager.h === --- lib/Format/WhitespaceManager.h +++ lib/Format/WhitespaceManager.h @@ -195,9 +195,7 @@ /// \brief Stores \p Text as the replacement for the whitespace in \p Range. void storeReplacement(SourceRange Range, StringRef Text); void appendNewlineText(std::string , unsigned Newlines); - void appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn); + void appendEscapedNewlineText(std::string , const Change ); void appendIndentText(std::string , unsigned IndentLevel, unsigned Spaces, unsigned WhitespaceStartColumn); Index: lib/Format/WhitespaceManager.cpp === --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -603,8 +603,7 @@ if (C.CreateReplacement) { std::string ReplacementText = C.PreviousLinePostfix; if (C.ContinuesPPDirective) -appendNewlineText(ReplacementText, C.NewlinesBefore, - C.PreviousEndOfTokenColumn, C.EscapedNewlineColumn); +appendEscapedNewlineText(ReplacementText, C); else appendNewlineText(ReplacementText, C.NewlinesBefore); appendIndentText(ReplacementText, C.Tok->IndentLevel, @@ -640,14 +639,19 @@ Text.append(UseCRLF ? "\r\n" : "\n"); } -void WhitespaceManager::appendNewlineText(std::string , unsigned Newlines, - unsigned PreviousEndOfTokenColumn, - unsigned EscapedNewlineColumn) { - if (Newlines > 0) { +void WhitespaceManager::appendEscapedNewlineText(std::string , const Change ) { + if (Style.AlignEscapedNewlines == FormatStyle::ENAS_DontAlign) { +for (unsigned i = 0; i < C.NewlinesBefore; ++i) + Text.append(UseCRLF ? " \\\r\n" : " \\\n"); +return; + } + + if (C.NewlinesBefore > 0) { +assert(C.EscapedNewlineColumn >= 1); unsigned Offset = -std::min(EscapedNewlineColumn - 2, PreviousEndOfTokenColumn); -for (unsigned i = 0; i < Newlines; ++i) { - Text.append(EscapedNewlineColumn - Offset - 1, ' '); +std::min(C.EscapedNewlineColumn - 1, C.PreviousEndOfTokenColumn); +for (unsigned i = 0; i < C.NewlinesBefore; ++i) { + Text.append(C.EscapedNewlineColumn - Offset - 1, ' '); Text.append(UseCRLF ? "\\\r\n" : "\\\n"); Offset = 0; } Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign =
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
djasper added a comment. Could you explain this in more detail? Which subtraction is underflowing? Why can't we just add a ternary expression there to handle the case? https://reviews.llvm.org/D36019 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D36019: [clang-format] Fix bug with ENAS_DontAlign and empty lines
jtbandes created this revision. Herald added a subscriber: klimek. This fixes a bug in `ENAS_DontAlign` (introduced in https://reviews.llvm.org/D32733) where blank lines had an EscapedNewlineColumn of 0, causing a subtraction to overflow when converted back to `unsigned` and leading to runaway memory allocation. This restores the original approach of a separate loop as originally proposed in https://reviews.llvm.org/D32733?vs=97397=97404, now with a proper justification :) https://reviews.llvm.org/D36019 Files: lib/Format/WhitespaceManager.cpp unittests/Format/FormatTest.cpp Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time it's reformatted + EXPECT_EQ("#define A \\\n" +" class Foo { \\\n" +"void bar(); \\\n" +" \\\n" +" \\\n" +" \\\n" +" public: \\\n" +"void baz(); \\\n" +" };", +format("#define A \\\n" + " class Foo { \\\n" + "void bar(); \\\n" + " \\\n" + " \\\n" + " \\\n" + " public: \\\n" + "void baz(); \\\n" + " };", DontAlign)); } TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) { Index: lib/Format/WhitespaceManager.cpp === --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -557,9 +557,22 @@ } void WhitespaceManager::alignEscapedNewlines() { - if (Style.AlignEscapedNewlines == FormatStyle::ENAS_DontAlign) + // If we are not aligning escaped newlines, just set EscapedNewlineColumn + // to point to the end of each line. + if (Style.AlignEscapedNewlines == FormatStyle::ENAS_DontAlign) { +bool PreviousContinuerHadNLBefore = false; // used to detect blank lines +for (Change : Changes) { + if (C.ContinuesPPDirective) { +if (C.NewlinesBefore > 0) + C.EscapedNewlineColumn = +PreviousContinuerHadNLBefore ? 2 : C.PreviousEndOfTokenColumn + 2; +PreviousContinuerHadNLBefore = C.NewlinesBefore > 0; + } +} return; + } + // Otherwise, compute the max width and then apply it to all lines. bool AlignLeft = Style.AlignEscapedNewlines == FormatStyle::ENAS_Left; unsigned MaxEndOfLine = AlignLeft ? 0 : Style.ColumnLimit; unsigned StartOfMacro = 0; @@ -644,6 +657,7 @@ unsigned PreviousEndOfTokenColumn, unsigned EscapedNewlineColumn) { if (Newlines > 0) { +assert(EscapedNewlineColumn >= 2); unsigned Offset = std::min(EscapedNewlineColumn - 2, PreviousEndOfTokenColumn); for (unsigned i = 0; i < Newlines; ++i) { Index: unittests/Format/FormatTest.cpp === --- unittests/Format/FormatTest.cpp +++ unittests/Format/FormatTest.cpp @@ -2309,6 +2309,30 @@ EXPECT_EQ("template f();", format("\\\ntemplate f();")); EXPECT_EQ("/* \\ \\ \\\n */", format("\\\n/* \\ \\ \\\n */")); EXPECT_EQ("", format("")); + + FormatStyle DontAlign = getLLVMStyle(); + DontAlign.AlignEscapedNewlines = FormatStyle::ENAS_DontAlign; + DontAlign.MaxEmptyLinesToKeep = 3; + // FIXME: can't use verifyFormat here because the newline before + // "public:" is not inserted the first time it's reformatted + EXPECT_EQ("#define A \\\n" +" class Foo { \\\n" +"void bar(); \\\n" +" \\\n" +" \\\n" +" \\\n" +" public: \\\n" +"void baz(); \\\n" +" };", +format("#define A \\\n" + " class Foo { \\\n" + "void bar(); \\\n" + " \\\n" + " \\\n" + " \\\n" + " public: \\\n" + "void baz(); \\\n" + " };", DontAlign)); } TEST_F(FormatTest, CalculateSpaceOnConsecutiveLinesInMacro) { Index: lib/Format/WhitespaceManager.cpp === --- lib/Format/WhitespaceManager.cpp +++ lib/Format/WhitespaceManager.cpp @@ -557,9 +557,22 @@ } void WhitespaceManager::alignEscapedNewlines() {