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); }

llvm-svn: 173243

clang/lib/Format/Format.cpp

@@ -112,7 +112,8 @@ class AnnotatedLine {
 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 @@ public:
   }
   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 @@ public:
   LineType Type;
   unsigned Level;
   bool InPPDirective;
+  bool MustBeDeclaration;
 };
 
 static prec::Level getPrecedence(const AnnotatedToken &Tok) {
@@ -1368,7 +1371,7 @@ private:
 
     // 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;

clang/lib/Format/UnwrappedLineParser.cpp

@@ -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::parse() {
 }
 
 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 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
       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 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) {
   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 @@ void UnwrappedLineParser::parseStructuralElement() {
     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 @@ void UnwrappedLineParser::parseStructuralElement() {
       // 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 @@ void UnwrappedLineParser::parseParens() {
       {
         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 @@ void UnwrappedLineParser::parseIfThenElse() {
     parseParens();
   bool NeedsUnwrappedLine = false;
   if (FormatTok.Tok.is(tok::l_brace)) {
-    parseBlock();
+    parseBlock(/*MustBeDeclaration=*/ false);
     NeedsUnwrappedLine = true;
   } else {
     addUnwrappedLine();
@@ -457,7 +482,7 @@ void UnwrappedLineParser::parseIfThenElse() {
   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 @@ void UnwrappedLineParser::parseNamespace() {
   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 @@ void UnwrappedLineParser::parseForOrWhileLoop() {
   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 @@ void UnwrappedLineParser::parseDoWhile() {
   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 @@ void UnwrappedLineParser::parseLabel() {
   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 @@ void UnwrappedLineParser::parseSwitch() {
   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 @@ void UnwrappedLineParser::parseRecord() {
     }
   }
   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 @@ void UnwrappedLineParser::parseObjCInterfaceOrImplementation() {
 
   // 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.

clang/lib/Format/UnwrappedLineParser.h

@@ -84,7 +84,7 @@ struct FormatToken {
 /// \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 @@ struct UnwrappedLine {
 
   /// \brief Whether this \c UnwrappedLine is part of a preprocessor directive.
   bool InPPDirective;
+
+  bool MustBeDeclaration;
 };
 
 class UnwrappedLineConsumer {
@@ -124,7 +126,7 @@ public:
 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 @@ private:
   // \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;

clang/unittests/Format/FormatTest.cpp

@@ -1666,6 +1666,11 @@ TEST_F(FormatTest, BlockComments) {
                    "/* */someCall(parameter);", getLLVMStyleWithColumns(15)));
 }
 
+TEST_F(FormatTest, Fuck) {
+  verifyFormat("void f(int *a);");
+  verifyFormat("void f() { f(fint * b); }");
+}
+
 //===----------------------------------------------------------------------===//
 // Objective-C tests.
 //===----------------------------------------------------------------------===//