r330324 - [clang-format] Don't remove empty lines before namespace endings

Krasimir Georgiev via cfe-commits cfe-commits at lists.llvm.org
Thu Apr 19 06:02:15 PDT 2018


Author: krasimir
Date: Thu Apr 19 06:02:15 2018
New Revision: 330324

URL: http://llvm.org/viewvc/llvm-project?rev=330324&view=rev
Log:
[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

Modified:
    cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
    cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h
    cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
    cfe/trunk/lib/Format/UnwrappedLineFormatter.h
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp?rev=330324&r1=330323&r2=330324&view=diff
==============================================================================
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp (original)
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp Thu Apr 19 06:02:15 2018
@@ -107,6 +107,7 @@ void updateEndComment(const FormatToken
                  << llvm::toString(std::move(Err)) << "\n";
   }
 }
+} // namespace
 
 const FormatToken *
 getNamespaceToken(const AnnotatedLine *line,
@@ -131,7 +132,6 @@ getNamespaceToken(const AnnotatedLine *l
     return nullptr;
   return NamespaceTok;
 }
-} // namespace
 
 NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment &Env,
                                                      const FormatStyle &Style)

Modified: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h?rev=330324&r1=330323&r2=330324&view=diff
==============================================================================
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h (original)
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h Thu Apr 19 06:02:15 2018
@@ -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<AnnotatedLine *> &AnnotatedLines);
+
 class NamespaceEndCommentsFixer : public TokenAnalyzer {
 public:
   NamespaceEndCommentsFixer(const Environment &Env, const FormatStyle &Style);

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=330324&r1=330323&r2=330324&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Thu Apr 19 06:02:15 2018
@@ -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 Sma
     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 Sma
                               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 Sma
   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<AnnotatedLine *> &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::formatFirst
   // 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)

Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.h?rev=330324&r1=330323&r2=330324&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.h Thu Apr 19 06:02:15 2018
@@ -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<AnnotatedLine *> &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.

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=330324&r1=330323&r2=330324&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Apr 19 06:02:15 2018
@@ -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"




More information about the cfe-commits mailing list