[cfe-commits] r171401 - in /cfe/trunk: lib/Format/Format.cpp unittests/Format/FormatTest.cpp

[Manuel Klimek]

Author: klimek
Date: Wed Jan  2 12:33:23 2013
New Revision: 171401

URL: http://llvm.org/viewvc/llvm-project?rev=171401&view=rev
Log:
Fixes multiple formatting bugs.

Fixes:
- incorrect handling of multiple consecutive preprocessor directives
- crash when trying to right align the escpaed newline for a line that
  is longer than the column limit
- using only ColumnLimit-1 columns when layouting with escaped newlines
  inside preprocessor directives

Modified:
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/Format.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=171401&r1=171400&r2=171401&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Jan  2 12:33:23 2013
@@ -237,6 +237,7 @@
     unsigned ParenLevel = State.Indent.size() - 1;
 
     if (Newline) {
+      unsigned WhitespaceStartColumn = State.Column;
       if (Current.Tok.is(tok::string_literal) &&
           Previous.Tok.is(tok::string_literal)) {
         State.Column = State.Column - Previous.Tok.getLength();
@@ -264,8 +265,12 @@
         State.LineContainsContinuedForLoopSection =
             Previous.Tok.isNot(tok::semi);
 
-      if (!DryRun)
-        replaceWhitespace(Current, 1, State.Column);
+      if (!DryRun) {
+        if (!Line.InPPDirective)
+          replaceWhitespace(Current, 1, State.Column);
+        else
+          replacePPWhitespace(Current, 1, State.Column, WhitespaceStartColumn);
+      }
 
       State.LastSpace[ParenLevel] = State.Indent[ParenLevel];
       if (Current.Tok.is(tok::colon) &&
@@ -398,7 +403,7 @@
     addTokenToState(NewLine, true, State);
 
     // Exceeding column limit is bad.
-    if (State.Column > Style.ColumnLimit)
+    if (State.Column > Style.ColumnLimit - (Line.InPPDirective ? 1 : 0))
       return UINT_MAX;
 
     if (StopAt <= CurrentPenalty)
@@ -434,12 +439,22 @@
   /// each \c FormatToken.
   void replaceWhitespace(const FormatToken &Tok, unsigned NewLines,
                          unsigned Spaces) {
+    Replaces.insert(tooling::Replacement(
+        SourceMgr, Tok.WhiteSpaceStart, Tok.WhiteSpaceLength,
+        std::string(NewLines, '\n') + std::string(Spaces, ' ')));
+  }
+
+  /// \brief Like \c replaceWhitespace, but additionally adds right-aligned
+  /// backslashes to escape newlines inside a preprocessor directive.
+  ///
+  /// This function and \c replaceWhitespace have the same behavior if
+  /// \c Newlines == 0.
+  void replacePPWhitespace(const FormatToken &Tok, unsigned NewLines,
+                           unsigned Spaces, unsigned WhitespaceStartColumn) {
     std::string NewLineText;
-    if (!Line.InPPDirective) {
-      NewLineText = std::string(NewLines, '\n');
-    } else if (NewLines > 0) {
+    if (NewLines > 0) {
       unsigned Offset =
-          SourceMgr.getSpellingColumnNumber(Tok.WhiteSpaceStart) - 1;
+          std::min<int>(Style.ColumnLimit - 1, WhitespaceStartColumn);
       for (unsigned i = 0; i < NewLines; ++i) {
         NewLineText += std::string(Style.ColumnLimit - Offset - 1, ' ');
         NewLineText += "\\\n";
@@ -469,7 +484,11 @@
          Token.Tok.is(tok::kw_private)) &&
         static_cast<int>(Indent) + Style.AccessModifierOffset >= 0)
       Indent += Style.AccessModifierOffset;
-    replaceWhitespace(Token, Newlines, Indent);
+    if (!Line.InPPDirective || Token.HasUnescapedNewline)
+      replaceWhitespace(Token, Newlines, Indent);
+    else
+      // FIXME: Figure out how to get the previous end-of-line column.
+      replacePPWhitespace(Token, Newlines, Indent, 0);
     return Indent;
   }
 

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=171401&r1=171400&r2=171401&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan  2 12:33:23 2013
@@ -64,12 +64,19 @@
     return MessedUp;
   }
 
-  void verifyFormat(llvm::StringRef Code) {
-    EXPECT_EQ(Code.str(), format(messUp(Code)));
+  FormatStyle getLLVMStyleWithColumns(unsigned ColumnLimit) {
+    FormatStyle Style = getLLVMStyle();
+    Style.ColumnLimit = ColumnLimit;
+    return Style;
+  }
+
+  void verifyFormat(llvm::StringRef Code,
+                    const FormatStyle &Style = getLLVMStyle()) {
+    EXPECT_EQ(Code.str(), format(messUp(Code), Style));
   }
 
   void verifyGoogleFormat(llvm::StringRef Code) {
-    EXPECT_EQ(Code.str(), format(messUp(Code), getGoogleStyle()));
+    verifyFormat(Code, getGoogleStyle());
   }
 };
 
@@ -396,6 +403,27 @@
   EXPECT_EQ("#define A B", format("#  \\\n define  \\\n    A  \\\n    B"));
 }
 
+TEST_F(FormatTest, IndentsPPDirectiveInReducedSpace) {
+  // If the macro fits in one line, we have the full width.
+  verifyFormat("#define A(B)", getLLVMStyleWithColumns(12));
+
+  verifyFormat("#define A(\\\n    B)", getLLVMStyleWithColumns(11));
+  verifyFormat("#define AA(\\\n    B)", getLLVMStyleWithColumns(11));
+  verifyFormat("#define A( \\\n    A, B)", getLLVMStyleWithColumns(12));
+}
+
+TEST_F(FormatTest, HandlePreprocessorDirectiveContext) {
+  verifyFormat(
+      "// some comment\n"
+      "\n"
+      "#include \"a.h\"\n"
+      "#define A(A,\\\n"
+      "          B)\n"
+      "#include \"b.h\"\n"
+      "\n"
+      "// some comment\n", getLLVMStyleWithColumns(13));
+}
+
 TEST_F(FormatTest, MixingPreprocessorDirectivesAndNormalCode) {
   verifyFormat("#define ALooooooooooooooooooooooooooooooooooooooongMacro("
                "                      \\\n"