From 621030523b0e57672979c495778ba8f13d86afa4 Mon Sep 17 00:00:00 2001 From: Krasimir Georgiev Date: Thu, 19 Apr 2018 13:02:15 +0000 Subject: [PATCH] [clang-format] Don't remove empty lines before namespace endings Summary: This implements an alternative to r327861, namely preserving empty lines before namespace endings. Reviewers: djasper Reviewed By: djasper Subscribers: klimek, cfe-commits Differential Revision: https://reviews.llvm.org/D45373 llvm-svn: 330324 --- clang/lib/Format/NamespaceEndCommentsFixer.cpp | 2 +- clang/lib/Format/NamespaceEndCommentsFixer.h | 10 ++++++++++ clang/lib/Format/UnwrappedLineFormatter.cpp | 18 ++++++++++-------- clang/lib/Format/UnwrappedLineFormatter.h | 5 +++-- clang/unittests/Format/FormatTest.cpp | 24 +++++++++++++++++++++++- 5 files changed, 47 insertions(+), 12 deletions(-) diff --git a/clang/lib/Format/NamespaceEndCommentsFixer.cpp b/clang/lib/Format/NamespaceEndCommentsFixer.cpp index ee11959..f5832a4 100644 --- a/clang/lib/Format/NamespaceEndCommentsFixer.cpp +++ b/clang/lib/Format/NamespaceEndCommentsFixer.cpp @@ -107,6 +107,7 @@ void updateEndComment(const FormatToken *RBraceTok, StringRef EndCommentText, << llvm::toString(std::move(Err)) << "\n"; } } +} // namespace const FormatToken * getNamespaceToken(const AnnotatedLine *line, @@ -131,7 +132,6 @@ getNamespaceToken(const AnnotatedLine *line, return nullptr; return NamespaceTok; } -} // namespace NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env, const FormatStyle &Style) diff --git a/clang/lib/Format/NamespaceEndCommentsFixer.h b/clang/lib/Format/NamespaceEndCommentsFixer.h index 4779f0d..adfe389 100644 --- a/clang/lib/Format/NamespaceEndCommentsFixer.h +++ b/clang/lib/Format/NamespaceEndCommentsFixer.h @@ -21,6 +21,16 @@ namespace clang { namespace format { +// Finds the namespace token corresponding to a closing namespace `}`, if that +// is to be formatted. +// If \p Line contains the closing `}` of a namespace, is affected and is not in +// a preprocessor directive, the result will be the matching namespace token. +// Otherwise returns null. +// \p AnnotatedLines is the sequence of lines from which \p Line is a member of. +const FormatToken * +getNamespaceToken(const AnnotatedLine *Line, + const SmallVectorImpl &AnnotatedLines); + class NamespaceEndCommentsFixer : public TokenAnalyzer { public: NamespaceEndCommentsFixer(const Environment &Env, const FormatStyle &Style); diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index 2ce39fb..953a5d3 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -7,6 +7,7 @@ // //===----------------------------------------------------------------------===// +#include "NamespaceEndCommentsFixer.h" #include "UnwrappedLineFormatter.h" #include "WhitespaceManager.h" #include "llvm/Support/Debug.h" @@ -1050,8 +1051,7 @@ UnwrappedLineFormatter::format(const SmallVectorImpl &Lines, if (ShouldFormat && TheLine.Type != LT_Invalid) { if (!DryRun) { bool LastLine = Line->First->is(tok::eof); - formatFirstToken(TheLine, PreviousLine, - Indent, + formatFirstToken(TheLine, PreviousLine, Lines, Indent, LastLine ? LastStartColumn : NextStartColumn + Indent); } @@ -1095,7 +1095,7 @@ UnwrappedLineFormatter::format(const SmallVectorImpl &Lines, TheLine.LeadingEmptyLinesAffected); // Format the first token. if (ReformatLeadingWhitespace) - formatFirstToken(TheLine, PreviousLine, + formatFirstToken(TheLine, PreviousLine, Lines, TheLine.First->OriginalColumn, TheLine.First->OriginalColumn); else @@ -1117,10 +1117,10 @@ UnwrappedLineFormatter::format(const SmallVectorImpl &Lines, return Penalty; } -void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line, - const AnnotatedLine *PreviousLine, - unsigned Indent, - unsigned NewlineIndent) { +void UnwrappedLineFormatter::formatFirstToken( + const AnnotatedLine &Line, const AnnotatedLine *PreviousLine, + const SmallVectorImpl &Lines, unsigned Indent, + unsigned NewlineIndent) { FormatToken &RootToken = *Line.First; if (RootToken.is(tok::eof)) { unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u); @@ -1134,7 +1134,9 @@ void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine &Line, // Remove empty lines before "}" where applicable. if (RootToken.is(tok::r_brace) && (!RootToken.Next || - (RootToken.Next->is(tok::semi) && !RootToken.Next->Next))) + (RootToken.Next->is(tok::semi) && !RootToken.Next->Next)) && + // Do not remove empty lines before namespace closing "}". + !getNamespaceToken(&Line, Lines)) Newlines = std::min(Newlines, 1u); // Remove empty lines at the start of nested blocks (lambdas/arrow functions) if (PreviousLine == nullptr && Line.Level > 0) diff --git a/clang/lib/Format/UnwrappedLineFormatter.h b/clang/lib/Format/UnwrappedLineFormatter.h index 6432ca8..756d99d 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.h +++ b/clang/lib/Format/UnwrappedLineFormatter.h @@ -49,8 +49,9 @@ private: /// \brief Add a new line and the required indent before the first Token /// of the \c UnwrappedLine if there was no structural parsing error. void formatFirstToken(const AnnotatedLine &Line, - const AnnotatedLine *PreviousLine, unsigned Indent, - unsigned NewlineIndent); + const AnnotatedLine *PreviousLine, + const SmallVectorImpl &Lines, + unsigned Indent, unsigned NewlineIndent); /// \brief Returns the column limit for a line, taking into account whether we /// need an escaped newline due to a continued preprocessor directive. diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 4397cdf..ddaa5ca 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -278,11 +278,12 @@ TEST_F(FormatTest, RemovesEmptyLines) { "\n" "}")); - // FIXME: This is slightly inconsistent. + // Don't remove empty lines before namespace endings. FormatStyle LLVMWithNoNamespaceFix = getLLVMStyle(); LLVMWithNoNamespaceFix.FixNamespaceComments = false; EXPECT_EQ("namespace {\n" "int i;\n" + "\n" "}", format("namespace {\n" "int i;\n" @@ -293,6 +294,27 @@ TEST_F(FormatTest, RemovesEmptyLines) { "}", format("namespace {\n" "int i;\n" + "}", LLVMWithNoNamespaceFix)); + EXPECT_EQ("namespace {\n" + "int i;\n" + "\n" + "};", + format("namespace {\n" + "int i;\n" + "\n" + "};", LLVMWithNoNamespaceFix)); + EXPECT_EQ("namespace {\n" + "int i;\n" + "};", + format("namespace {\n" + "int i;\n" + "};", LLVMWithNoNamespaceFix)); + EXPECT_EQ("namespace {\n" + "int i;\n" + "\n" + "}", + format("namespace {\n" + "int i;\n" "\n" "}")); EXPECT_EQ("namespace {\n" -- 2.7.4