[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings

[Krasimir Georgiev via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
krasimir marked an inline comment as done.
Closed by commit rL330324: [clang-format] Dont remove empty lines before 
namespace endings (authored by krasimir, committed by ).
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D45373?vs=141352=143081#toc

Repository:
  rL LLVM

https://reviews.llvm.org/D45373

Files:
  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

Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
===
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.cpp
@@ -107,6 +107,7 @@
  << llvm::toString(std::move(Err)) << "\n";
   }
 }
+} // namespace
 
 const FormatToken *
 getNamespaceToken(const AnnotatedLine *line,
@@ -131,7 +132,6 @@
 return nullptr;
   return NamespaceTok;
 }
-} // namespace
 
 NamespaceEndCommentsFixer::NamespaceEndCommentsFixer(const Environment ,
  const FormatStyle )
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.h
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.h
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.h
@@ -49,8 +49,9 @@
   /// \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 ,
-const AnnotatedLine *PreviousLine, unsigned Indent,
-unsigned NewlineIndent);
+const AnnotatedLine *PreviousLine,
+const SmallVectorImpl ,
+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.
Index: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp
+++ cfe/trunk/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 @@
 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 @@
   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 @@
   return Penalty;
 }
 
-void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine ,
-  const AnnotatedLine *PreviousLine,
-  unsigned Indent,
-  unsigned NewlineIndent) {
+void UnwrappedLineFormatter::formatFirstToken(
+const AnnotatedLine , const AnnotatedLine *PreviousLine,
+const SmallVectorImpl , unsigned Indent,
+unsigned NewlineIndent) {
   FormatToken  = *Line.First;
   if (RootToken.is(tok::eof)) {
 unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u);
@@ -1134,7 +1134,9 @@
   // 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(, Lines))
 Newlines = std::min(Newlines, 1u);
   // Remove empty lines at the start of nested blocks (lambdas/arrow functions)
   if (PreviousLine == nullptr && Line.Level > 0)
Index: cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h
===
--- cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h
+++ cfe/trunk/lib/Format/NamespaceEndCommentsFixer.h
@@ -21,6 +21,16 @@
 namespace clang {
 namespace format {
 
+// Finds the 

[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir marked an inline comment as done.
krasimir added inline comments.



Comment at: lib/Format/NamespaceEndCommentsFixer.h:24
 
+// Finds the namespace token corresponding to a closing namespace `}`, if that
+// is to be formatted.

djasper wrote:
> I don't understand the "if that is to be formatted part". What do you mean?
I'm documenting the current behavior of the implementation, getting into more 
detail in the next paragraph of the comments: this returns `nullptr` if `Line` 
is not `Affected`, even if otherwise would have a namespace token.


Repository:
  rC Clang

https://reviews.llvm.org/D45373



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings

[Daniel Jasper via Phabricator via cfe-commits]

djasper accepted this revision.
djasper added a comment.
This revision is now accepted and ready to land.

Looks good.




Comment at: lib/Format/NamespaceEndCommentsFixer.h:24
 
+// Finds the namespace token corresponding to a closing namespace `}`, if that
+// is to be formatted.

I don't understand the "if that is to be formatted part". What do you mean?


Repository:
  rC Clang

https://reviews.llvm.org/D45373



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45373: [clang-format] Don't remove empty lines before namespace endings

[Krasimir Georgiev via Phabricator via cfe-commits]

krasimir created this revision.
krasimir added a reviewer: djasper.
Herald added subscribers: cfe-commits, klimek.

This implements an alternative to r327861, namely preserving empty lines before 
namespace endings.


Repository:
  rC Clang

https://reviews.llvm.org/D45373

Files:
  lib/Format/NamespaceEndCommentsFixer.cpp
  lib/Format/NamespaceEndCommentsFixer.h
  lib/Format/UnwrappedLineFormatter.cpp
  lib/Format/UnwrappedLineFormatter.h
  unittests/Format/FormatTest.cpp

Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -278,19 +278,41 @@
"\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"
"\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"
+   "};", 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"
Index: lib/Format/UnwrappedLineFormatter.h
===
--- lib/Format/UnwrappedLineFormatter.h
+++ lib/Format/UnwrappedLineFormatter.h
@@ -49,8 +49,9 @@
   /// \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 ,
-const AnnotatedLine *PreviousLine, unsigned Indent,
-unsigned NewlineIndent);
+const AnnotatedLine *PreviousLine,
+const SmallVectorImpl ,
+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.
Index: lib/Format/UnwrappedLineFormatter.cpp
===
--- lib/Format/UnwrappedLineFormatter.cpp
+++ 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 @@
 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 @@
   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 @@
   return Penalty;
 }
 
-void UnwrappedLineFormatter::formatFirstToken(const AnnotatedLine ,
-  const AnnotatedLine *PreviousLine,
-  unsigned Indent,
-  unsigned NewlineIndent) {
+void UnwrappedLineFormatter::formatFirstToken(
+const AnnotatedLine , const AnnotatedLine *PreviousLine,
+const SmallVectorImpl , unsigned Indent,
+unsigned NewlineIndent) {
   FormatToken  = *Line.First;
   if (RootToken.is(tok::eof)) {
 unsigned Newlines = std::min(RootToken.NewlinesBefore, 1u);
@@ -1134,7 +1134,9 @@
   // 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) &&