[cfe-commits] r173243 - in /cfe/trunk: lib/Format/Format.cpp lib/Format/UnwrappedLineParser.cpp lib/Format/UnwrappedLineParser.h unittests/Format/FormatTest.cpp

Manuel Klimek klimek at google.com
Wed Jan 23 01:32:49 PST 2013


Author: klimek
Date: Wed Jan 23 03:32:48 2013
New Revision: 173243

URL: http://llvm.org/viewvc/llvm-project?rev=173243&view=rev
Log:
Allow us to better guess the context of an unwrapped line.

This gives us the ability to guess better defaults for whether a *
between identifiers is a pointer dereference or binary operator.

Now correctly formats:
void f(a *b);
void f() { f(a * b); }

Modified:
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/lib/Format/UnwrappedLineParser.cpp
    cfe/trunk/lib/Format/UnwrappedLineParser.h
    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=173243&r1=173242&r2=173243&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Jan 23 03:32:48 2013
@@ -112,7 +112,8 @@
 public:
   AnnotatedLine(const UnwrappedLine &Line)
       : First(Line.Tokens.front()), Level(Line.Level),
-        InPPDirective(Line.InPPDirective) {
+        InPPDirective(Line.InPPDirective),
+        MustBeDeclaration(Line.MustBeDeclaration) {
     assert(!Line.Tokens.empty());
     AnnotatedToken *Current = &First;
     for (std::list<FormatToken>::const_iterator I = ++Line.Tokens.begin(),
@@ -126,7 +127,8 @@
   }
   AnnotatedLine(const AnnotatedLine &Other)
       : First(Other.First), Type(Other.Type), Level(Other.Level),
-        InPPDirective(Other.InPPDirective) {
+        InPPDirective(Other.InPPDirective),
+        MustBeDeclaration(Other.MustBeDeclaration) {
     Last = &First;
     while (!Last->Children.empty()) {
       Last->Children[0].Parent = Last;
@@ -140,6 +142,7 @@
   LineType Type;
   unsigned Level;
   bool InPPDirective;
+  bool MustBeDeclaration;
 };
 
 static prec::Level getPrecedence(const AnnotatedToken &Tok) {
@@ -1368,7 +1371,7 @@
 
     // It is very unlikely that we are going to find a pointer or reference type
     // definition on the RHS of an assignment.
-    if (IsRHS)
+    if (IsRHS || !Line.MustBeDeclaration)
       return TT_BinaryOperator;
 
     return TT_PointerOrReference;

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=173243&r1=173242&r2=173243&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Wed Jan 23 03:32:48 2013
@@ -28,6 +28,23 @@
 namespace clang {
 namespace format {
 
+class ScopedDeclarationState {
+public:
+  ScopedDeclarationState(UnwrappedLine &Line, std::vector<bool> &Stack,
+                         bool MustBeDeclaration)
+      : Line(Line), Stack(Stack) {
+    Stack.push_back(MustBeDeclaration);
+    Line.MustBeDeclaration = MustBeDeclaration;
+  }
+  ~ScopedDeclarationState() {
+    Line.MustBeDeclaration = Stack.back();
+    Stack.pop_back();
+  }
+private:
+  UnwrappedLine &Line;
+  std::vector<bool> &Stack;
+};
+
 class ScopedMacroState : public FormatTokenSource {
 public:
   ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource,
@@ -128,6 +145,8 @@
 }
 
 bool UnwrappedLineParser::parseFile() {
+  ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
+                                          /*MustBeDeclaration=*/ true);
   bool Error = parseLevel(/*HasOpeningBrace=*/false);
   // Make sure to format the remaining tokens.
   flushComments(true);
@@ -144,7 +163,9 @@
       addUnwrappedLine();
       break;
     case tok::l_brace:
-      Error |= parseBlock();
+      // FIXME: Add parameter whether this can happen - if this happens, we must
+      // be in a non-declaration context.
+      Error |= parseBlock(/*MustBeDeclaration=*/ false);
       addUnwrappedLine();
       break;
     case tok::r_brace:
@@ -167,12 +188,14 @@
   return Error;
 }
 
-bool UnwrappedLineParser::parseBlock(unsigned AddLevels) {
+bool UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels) {
   assert(FormatTok.Tok.is(tok::l_brace) && "'{' expected");
   nextToken();
 
   addUnwrappedLine();
 
+  ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
+                                          MustBeDeclaration);
   Line->Level += AddLevels;
   parseLevel(/*HasOpeningBrace=*/true);
 
@@ -307,7 +330,7 @@
     if (FormatTok.Tok.is(tok::string_literal)) {
       nextToken();
       if (FormatTok.Tok.is(tok::l_brace)) {
-        parseBlock(0);
+        parseBlock(/*MustBeDeclaration=*/ true, 0);
         addUnwrappedLine();
         return;
       }
@@ -345,7 +368,7 @@
       // structural element.
       // FIXME: Figure out cases where this is not true, and add projections for
       // them (the one we know is missing are lambdas).
-      parseBlock();
+      parseBlock(/*MustBeDeclaration=*/ false);
       addUnwrappedLine();
       return;
     case tok::identifier:
@@ -427,8 +450,10 @@
       {
         nextToken();
         ScopedLineState LineState(*this);
+        ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack,
+                                                /*MustBeDeclaration=*/ false);
         Line->Level += 1;
-        parseLevel(/*HasOpeningBrace=*/true);
+        parseLevel(/*HasOpeningBrace=*/ true);
         Line->Level -= 1;
       }
       break;
@@ -446,7 +471,7 @@
     parseParens();
   bool NeedsUnwrappedLine = false;
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ false);
     NeedsUnwrappedLine = true;
   } else {
     addUnwrappedLine();
@@ -457,7 +482,7 @@
   if (FormatTok.Tok.is(tok::kw_else)) {
     nextToken();
     if (FormatTok.Tok.is(tok::l_brace)) {
-      parseBlock();
+      parseBlock(/*MustBeDeclaration=*/ false);
       addUnwrappedLine();
     } else if (FormatTok.Tok.is(tok::kw_if)) {
       parseIfThenElse();
@@ -478,7 +503,7 @@
   if (FormatTok.Tok.is(tok::identifier))
     nextToken();
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock(0);
+    parseBlock(/*MustBeDeclaration=*/ true, 0);
     addUnwrappedLine();
   }
   // FIXME: Add error handling.
@@ -491,7 +516,7 @@
   if (FormatTok.Tok.is(tok::l_paren))
     parseParens();
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ false);
     addUnwrappedLine();
   } else {
     addUnwrappedLine();
@@ -505,7 +530,7 @@
   assert(FormatTok.Tok.is(tok::kw_do) && "'do' expected");
   nextToken();
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ false);
   } else {
     addUnwrappedLine();
     ++Line->Level;
@@ -531,7 +556,7 @@
   if (Line->Level > 0)
     --Line->Level;
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ false);
     if (FormatTok.Tok.is(tok::kw_break))
       parseStructuralElement(); // "break;" after "}" goes on the same line.
   }
@@ -554,7 +579,7 @@
   if (FormatTok.Tok.is(tok::l_paren))
     parseParens();
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock(Style.IndentCaseLabels ? 2 : 1);
+    parseBlock(/*MustBeDeclaration=*/ false, Style.IndentCaseLabels ? 2 : 1);
     addUnwrappedLine();
   } else {
     addUnwrappedLine();
@@ -648,7 +673,7 @@
     }
   }
   if (FormatTok.Tok.is(tok::l_brace))
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ true);
   // We fall through to parsing a structural element afterwards, so
   // class A {} n, m;
   // will end up in one unwrapped line.
@@ -690,7 +715,7 @@
 
   // If instance variables are present, keep the '{' on the first line too.
   if (FormatTok.Tok.is(tok::l_brace))
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ true);
 
   // With instance variables, this puts '}' on its own line.  Without instance
   // variables, this ends the @interface line.

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.h?rev=173243&r1=173242&r2=173243&view=diff
==============================================================================
--- cfe/trunk/lib/Format/UnwrappedLineParser.h (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.h Wed Jan 23 03:32:48 2013
@@ -84,7 +84,7 @@
 /// \c UnwrappedLineFormatter. The key property is that changing the formatting
 /// within an unwrapped line does not affect any other unwrapped lines.
 struct UnwrappedLine {
-  UnwrappedLine() : Level(0), InPPDirective(false) {
+  UnwrappedLine() : Level(0), InPPDirective(false), MustBeDeclaration(false) {
   }
 
   // FIXME: Don't use std::list here.
@@ -96,6 +96,8 @@
 
   /// \brief Whether this \c UnwrappedLine is part of a preprocessor directive.
   bool InPPDirective;
+
+  bool MustBeDeclaration;
 };
 
 class UnwrappedLineConsumer {
@@ -124,7 +126,7 @@
 private:
   bool parseFile();
   bool parseLevel(bool HasOpeningBrace);
-  bool parseBlock(unsigned AddLevels = 1);
+  bool parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1);
   void parsePPDirective();
   void parsePPDefine();
   void parsePPUnknown();
@@ -180,6 +182,10 @@
   // \c &PreprocessorDirectives.
   std::vector<UnwrappedLine> *CurrentLines;
 
+  // We store for each line whether it must be a declaration depending on
+  // whether we are in a compound statement or not.
+  std::vector<bool> DeclarationScopeStack;
+
   clang::DiagnosticsEngine &Diag;
   const FormatStyle &Style;
   FormatTokenSource *Tokens;

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=173243&r1=173242&r2=173243&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Jan 23 03:32:48 2013
@@ -1666,6 +1666,11 @@
                    "/* */someCall(parameter);", getLLVMStyleWithColumns(15)));
 }
 
+TEST_F(FormatTest, Fuck) {
+  verifyFormat("void f(int *a);");
+  verifyFormat("void f() { f(fint * b); }");
+}
+
 //===----------------------------------------------------------------------===//
 // Objective-C tests.
 //===----------------------------------------------------------------------===//





More information about the cfe-commits mailing list