[PATCH] Formatter: Correctly format stars in `sizeof(int**)` and similar places.

[Nico Weber]

Hi djasper,

This redoes how '*' and '&' are classified as pointer / reference markers when followed by ')', '>', or ','.

Previously, determineStarAmpUsage() marked a single '*' and '&' followed by ')', '>', or ',' as pointer or reference marker. Now, all '*'s and '&'s preceding ')', '>', or ',' are marked as pointer / reference markers. Fixes PR14884.

Since only the last '*' in 'int ***' was marked as pointer before (the rest were unary operators, which don't reach spaceRequiredBetween()), spaceRequiredBetween() now had to be thought about handing multiple '*'s in sequence.

Before:
  return sizeof(int * *);
  Type **A = static_cast<Type * *>(P);

Now:
  return sizeof(int**);
  Type **A = static_cast<Type **>(P);

While here, also make all methods of AnnotatingParser except parseLine() private.

http://llvm-reviews.chandlerc.com/D384

Files:
  unittests/Format/FormatTest.cpp
  lib/Format/TokenAnnotator.cpp

Index: unittests/Format/FormatTest.cpp
===================================================================
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -1588,6 +1588,14 @@
   verifyIndependentOfContext("a * (a + b);");
   verifyIndependentOfContext("(a *)(a + b);");
   verifyIndependentOfContext("int *pa = (int *)&a;");
+  verifyIndependentOfContext("return sizeof(int **);");
+  verifyIndependentOfContext("return sizeof(int ******);");
+  verifyIndependentOfContext("return (int **&)a;");
+  verifyGoogleFormat("return sizeof(int**);");
+  verifyIndependentOfContext("Type **A = static_cast<Type **>(P);");
+  verifyGoogleFormat("Type** A = static_cast<Type**>(P);");
+  // FIXME: The newline is wrong.
+  verifyFormat("auto a = [](int **&, int ***) {}\n;");
 
   verifyIndependentOfContext("InvalidRegions[*R] = 0;");
 
@@ -2509,8 +2517,7 @@
   verifyFormat("@dynamic textColor;");
   verifyFormat("char *buf1 = @encode(int *);");
   verifyFormat("char *buf1 = @encode(typeof(4 * 5));");
-  // FIXME: Enable once PR14884 is fixed:
-  //verifyFormat("char *buf1 = @encode(int **);");
+  verifyFormat("char *buf1 = @encode(int **);");
   verifyFormat("Protocol *proto = @protocol(p1);");
   verifyFormat("SEL s = @selector(foo:);");
   verifyFormat("@synchronized(self) {\n"
Index: lib/Format/TokenAnnotator.cpp
===================================================================
--- lib/Format/TokenAnnotator.cpp
+++ lib/Format/TokenAnnotator.cpp
@@ -85,6 +85,7 @@
     Contexts.back().LookForFunctionName = Line.MustBeDeclaration;
   }
 
+private:
   bool parseAngle() {
     if (CurrentToken == NULL)
       return false;
@@ -442,6 +443,7 @@
       next();
   }
 
+public:
   LineType parseLine() {
     int PeriodsAndArrows = 0;
     bool CanBeBuilderTypeStmt = true;
@@ -477,6 +479,7 @@
     return LT_Other;
   }
 
+private:
   void next() {
     if (CurrentToken != NULL) {
       determineTokenType(*CurrentToken);
@@ -489,7 +492,6 @@
       CurrentToken = NULL;
   }
 
-private:
   /// \brief A struct to hold information valid in a specific context, e.g.
   /// a pair of parenthesis.
   struct Context {
@@ -527,19 +529,25 @@
   void determineTokenType(AnnotatedToken &Current) {
     if (getPrecedence(Current) == prec::Assignment) {
       Contexts.back().IsExpression = true;
-      AnnotatedToken *Previous = Current.Parent;
-      while (Previous != NULL && Previous->isNot(tok::comma)) {
+      for (AnnotatedToken *Previous = Current.Parent;
+           Previous && Previous->isNot(tok::comma);
+           Previous = Previous->Parent) {
         if (Previous->Type == TT_BinaryOperator &&
             (Previous->is(tok::star) || Previous->is(tok::amp))) {
           Previous->Type = TT_PointerOrReference;
         }
-        Previous = Previous->Parent;
       }
-    }
-    if (Current.is(tok::kw_return) || Current.is(tok::kw_throw) ||
+    } else if (Current.is(tok::kw_return) || Current.is(tok::kw_throw) ||
         (Current.is(tok::l_paren) && !Line.MustBeDeclaration &&
-         (Current.Parent == NULL || Current.Parent->isNot(tok::kw_for))))
+         (Current.Parent == NULL || Current.Parent->isNot(tok::kw_for)))) {
       Contexts.back().IsExpression = true;
+    } else if (Current.is(tok::r_paren) || Current.is(tok::greater) ||
+               Current.is(tok::comma)) {
+      for (AnnotatedToken *Previous = Current.Parent;
+           Previous && (Previous->is(tok::star) || Previous->is(tok::amp));
+           Previous = Previous->Parent)
+        Previous->Type = TT_PointerOrReference;
+    }
 
     if (Current.Type == TT_Unknown) {
       if (Contexts.back().LookForFunctionName && Current.is(tok::l_paren)) {
@@ -633,10 +641,6 @@
         NextToken->is(tok::l_square))
       return TT_BinaryOperator;
 
-    if (NextToken->is(tok::comma) || NextToken->is(tok::r_paren) ||
-        NextToken->is(tok::greater))
-      return TT_PointerOrReference;
-
     // It is very unlikely that we are going to find a pointer or reference type
     // definition on the RHS of an assignment.
     if (IsExpression)
@@ -921,9 +925,11 @@
   if (Right.is(tok::amp) || Right.is(tok::star))
     return Left.FormatTok.Tok.isLiteral() ||
            (Left.isNot(tok::star) && Left.isNot(tok::amp) &&
-            !Style.PointerBindsToType);
+            Left.isNot(tok::l_paren) && !Style.PointerBindsToType);
   if (Left.is(tok::amp) || Left.is(tok::star))
-    return Right.FormatTok.Tok.isLiteral() || Style.PointerBindsToType;
+    return Right.FormatTok.Tok.isLiteral() ||
+           (Right.isNot(tok::star) && Right.isNot(tok::amp) &&
+            Style.PointerBindsToType);
   if (Right.is(tok::star) && Left.is(tok::l_paren))
     return false;
   if (Left.is(tok::l_square) || Right.is(tok::r_square))
-------------- next part --------------
A non-text attachment was scrubbed...
Name: D384.1.patch
Type: text/x-patch
Size: 4776 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130209/2a8b7379/attachment.bin>