r179167 - Fix labels with trailing comments and cleanup.

Daniel Jasper djasper at google.com
Wed Apr 10 02:49:49 PDT 2013


Author: djasper
Date: Wed Apr 10 04:49:49 2013
New Revision: 179167

URL: http://llvm.org/viewvc/llvm-project?rev=179167&view=rev
Log:
Fix labels with trailing comments and cleanup.

Before:
class A {
public : // test
};

After:
class A {
public: // test
};

Also remove duplicate methods calculating properties of AnnotatedTokens
and make them members of AnnotatedTokens so that they are in a common
place.

Modified:
    cfe/trunk/lib/Format/Format.cpp
    cfe/trunk/lib/Format/TokenAnnotator.cpp
    cfe/trunk/lib/Format/TokenAnnotator.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=179167&r1=179166&r2=179167&view=diff
==============================================================================
--- cfe/trunk/lib/Format/Format.cpp (original)
+++ cfe/trunk/lib/Format/Format.cpp Wed Apr 10 04:49:49 2013
@@ -81,11 +81,6 @@ FormatStyle getChromiumStyle() {
   return ChromiumStyle;
 }
 
-static bool isTrailingComment(const AnnotatedToken &Tok) {
-  return Tok.is(tok::comment) &&
-         (Tok.Children.empty() || Tok.Children[0].MustBreakBefore);
-}
-
 // Returns the length of everything up to the first possible line break after
 // the ), ], } or > matching \c Tok.
 static unsigned getLengthToMatchingParen(const AnnotatedToken &Tok) {
@@ -127,7 +122,7 @@ public:
 
     // Align line comments if they are trailing or if they continue other
     // trailing comments.
-    if (isTrailingComment(Tok)) {
+    if (Tok.isTrailingComment()) {
       // Remove the comment's trailing whitespace.
       if (Tok.FormatTok.Tok.getLength() != Tok.FormatTok.TokenLength)
         Replaces.insert(tooling::Replacement(
@@ -151,7 +146,7 @@ public:
     }
 
     // If this line does not have a trailing comment, align the stored comments.
-    if (Tok.Children.empty() && !isTrailingComment(Tok))
+    if (Tok.Children.empty() && !Tok.isTrailingComment())
       alignComments();
 
     if (Tok.Type == TT_BlockComment) {
@@ -819,10 +814,10 @@ private:
               State.Column + Spaces + Current.FormatTok.TokenLength;
       }
 
-      if (opensScope(Previous) && Previous.Type != TT_ObjCMethodExpr &&
+      if (Previous.opensScope() && Previous.Type != TT_ObjCMethodExpr &&
           Current.Type != TT_LineComment)
         State.Stack.back().Indent = State.Column + Spaces;
-      if (Previous.is(tok::comma) && !isTrailingComment(Current))
+      if (Previous.is(tok::comma) && !Current.isTrailingComment())
         State.Stack.back().HasMultiParameterLine = true;
 
       State.Column += Spaces;
@@ -839,7 +834,7 @@ private:
         State.Stack.back().LastSpace = State.Column;
       else if (Previous.Type == TT_InheritanceColon)
         State.Stack.back().Indent = State.Column;
-      else if (opensScope(Previous) && Previous.ParameterCount > 1)
+      else if (Previous.opensScope() && Previous.ParameterCount > 1)
         // If this function has multiple parameters, indent nested calls from
         // the start of the first parameter.
         State.Stack.back().LastSpace = State.Column;
@@ -887,7 +882,7 @@ private:
     // is special cased.
     bool SkipFirstExtraIndent =
         Current.is(tok::kw_return) ||
-        (Previous && (opensScope(*Previous) ||
+        (Previous && (Previous->opensScope() ||
                       getPrecedence(*Previous) == prec::Assignment));
     for (SmallVector<prec::Level, 4>::const_reverse_iterator
              I = Current.FakeLParens.rbegin(),
@@ -905,7 +900,7 @@ private:
       if (*I == prec::Conditional ||
           (!SkipFirstExtraIndent && *I > prec::Assignment))
         NewParenState.Indent += 4;
-      if (Previous && !opensScope(*Previous))
+      if (Previous && !Previous->opensScope())
         NewParenState.BreakBeforeParameter = false;
       State.Stack.push_back(NewParenState);
       SkipFirstExtraIndent = false;
@@ -913,7 +908,7 @@ private:
 
     // If we encounter an opening (, [, { or <, we add a level to our stacks to
     // prepare for the following tokens.
-    if (opensScope(Current)) {
+    if (Current.opensScope()) {
       unsigned NewIndent;
       bool AvoidBinPacking;
       if (Current.is(tok::l_brace)) {
@@ -1229,7 +1224,7 @@ private:
          State.NextToken->is(tok::question) ||
          State.NextToken->Type == TT_ConditionalExpr) &&
         State.Stack.back().BreakBeforeParameter &&
-        !isTrailingComment(*State.NextToken) &&
+        !State.NextToken->isTrailingComment() &&
         State.NextToken->isNot(tok::r_paren) &&
         State.NextToken->isNot(tok::r_brace))
       return true;

Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=179167&r1=179166&r2=179167&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Wed Apr 10 04:49:49 2013
@@ -21,8 +21,8 @@
 namespace clang {
 namespace format {
 
-static bool isUnaryOperator(const AnnotatedToken &Tok) {
-  switch (Tok.FormatTok.Tok.getKind()) {
+bool AnnotatedToken::isUnaryOperator() const {
+  switch (FormatTok.Tok.getKind()) {
   case tok::plus:
   case tok::plusplus:
   case tok::minus:
@@ -37,49 +37,38 @@ static bool isUnaryOperator(const Annota
   }
 }
 
-static bool isBinaryOperator(const AnnotatedToken &Tok) {
+bool AnnotatedToken::isBinaryOperator() const {
   // Comma is a binary operator, but does not behave as such wrt. formatting.
-  return getPrecedence(Tok) > prec::Comma;
+  return getPrecedence(*this) > prec::Comma;
 }
 
-// Returns the previous token ignoring comments.
-static AnnotatedToken *getPreviousToken(AnnotatedToken &Tok) {
-  AnnotatedToken *PrevToken = Tok.Parent;
-  while (PrevToken != NULL && PrevToken->is(tok::comment))
-    PrevToken = PrevToken->Parent;
-  return PrevToken;
-}
-static const AnnotatedToken *getPreviousToken(const AnnotatedToken &Tok) {
-  return getPreviousToken(const_cast<AnnotatedToken &>(Tok));
+bool AnnotatedToken::isTrailingComment() const {
+  return is(tok::comment) &&
+         (Children.empty() || Children[0].FormatTok.NewlinesBefore > 0);
 }
 
-static bool isTrailingComment(AnnotatedToken *Tok) {
-  return Tok != NULL && Tok->is(tok::comment) &&
-         (Tok->Children.empty() ||
-          Tok->Children[0].FormatTok.NewlinesBefore > 0);
+AnnotatedToken *AnnotatedToken::getPreviousNoneComment() const {
+  AnnotatedToken *Tok = Parent;
+  while (Tok != NULL && Tok->is(tok::comment))
+    Tok = Tok->Parent;
+  return Tok;
 }
 
-// Returns the next token ignoring comments.
-static const AnnotatedToken *getNextToken(const AnnotatedToken &Tok) {
-  if (Tok.Children.empty())
-    return NULL;
-  const AnnotatedToken *NextToken = &Tok.Children[0];
-  while (NextToken->is(tok::comment)) {
-    if (NextToken->Children.empty())
-      return NULL;
-    NextToken = &NextToken->Children[0];
-  }
-  return NextToken;
+const AnnotatedToken *AnnotatedToken::getNextNoneComment() const {
+  const AnnotatedToken *Tok = Children.empty() ? NULL : &Children[0];
+  while (Tok != NULL && Tok->is(tok::comment))
+    Tok = Tok->Children.empty() ? NULL : &Tok->Children[0];
+  return Tok;
 }
 
-bool closesScope(const AnnotatedToken &Tok) {
-  return Tok.isOneOf(tok::r_paren, tok::r_brace, tok::r_square) ||
-         Tok.Type == TT_TemplateCloser;
+bool AnnotatedToken::closesScope() const {
+  return isOneOf(tok::r_paren, tok::r_brace, tok::r_square) ||
+         Type == TT_TemplateCloser;
 }
 
-bool opensScope(const AnnotatedToken &Tok) {
-  return Tok.isOneOf(tok::l_paren, tok::l_brace, tok::l_square) ||
-         Tok.Type == TT_TemplateOpener;
+bool AnnotatedToken::opensScope() const {
+  return isOneOf(tok::l_paren, tok::l_brace, tok::l_square) ||
+         Type == TT_TemplateOpener;
 }
 
 /// \brief A parser that gathers additional information about tokens.
@@ -197,12 +186,12 @@ private:
     // ')' or ']'), it could be the start of an Objective-C method
     // expression, or it could the the start of an Objective-C array literal.
     AnnotatedToken *Left = CurrentToken->Parent;
-    AnnotatedToken *Parent = getPreviousToken(*Left);
+    AnnotatedToken *Parent = Left->getPreviousNoneComment();
     bool StartsObjCMethodExpr =
         Contexts.back().CanBeExpression &&
         (!Parent || Parent->isOneOf(tok::colon, tok::l_square, tok::l_paren,
                                     tok::kw_return, tok::kw_throw) ||
-         isUnaryOperator(*Parent) || Parent->Type == TT_ObjCForIn ||
+         Parent->isUnaryOperator() || Parent->Type == TT_ObjCForIn ||
          Parent->Type == TT_CastRParen ||
          getBinOpPrecedence(Parent->FormatTok.Tok.getKind(), true, true) >
              prec::Unknown);
@@ -621,7 +610,7 @@ private:
         Current.Type = determineIncrementUsage(Current);
       } else if (Current.is(tok::exclaim)) {
         Current.Type = TT_UnaryOperator;
-      } else if (isBinaryOperator(Current)) {
+      } else if (Current.isBinaryOperator()) {
         Current.Type = TT_BinaryOperator;
       } else if (Current.is(tok::comment)) {
         std::string Data(Lexer::getSpelling(Current.FormatTok.Tok, SourceMgr,
@@ -665,11 +654,11 @@ private:
   /// \brief Return the type of the given token assuming it is * or &.
   TokenType
   determineStarAmpUsage(const AnnotatedToken &Tok, bool IsExpression) {
-    const AnnotatedToken *PrevToken = getPreviousToken(Tok);
+    const AnnotatedToken *PrevToken = Tok.getPreviousNoneComment();
     if (PrevToken == NULL)
       return TT_UnaryOperator;
 
-    const AnnotatedToken *NextToken = getNextToken(Tok);
+    const AnnotatedToken *NextToken = Tok.getNextNoneComment();
     if (NextToken == NULL)
       return TT_Unknown;
 
@@ -688,7 +677,7 @@ private:
 
     if (PrevToken->FormatTok.Tok.isLiteral() ||
         PrevToken->isOneOf(tok::r_paren, tok::r_square) ||
-        NextToken->FormatTok.Tok.isLiteral() || isUnaryOperator(*NextToken))
+        NextToken->FormatTok.Tok.isLiteral() || NextToken->isUnaryOperator())
       return TT_BinaryOperator;
 
     // It is very unlikely that we are going to find a pointer or reference type
@@ -700,7 +689,7 @@ private:
   }
 
   TokenType determinePlusMinusCaretUsage(const AnnotatedToken &Tok) {
-    const AnnotatedToken *PrevToken = getPreviousToken(Tok);
+    const AnnotatedToken *PrevToken = Tok.getPreviousNoneComment();
     if (PrevToken == NULL)
       return TT_UnaryOperator;
 
@@ -720,7 +709,7 @@ private:
 
   /// \brief Determine whether ++/-- are pre- or post-increments/-decrements.
   TokenType determineIncrementUsage(const AnnotatedToken &Tok) {
-    const AnnotatedToken *PrevToken = getPreviousToken(Tok);
+    const AnnotatedToken *PrevToken = Tok.getPreviousNoneComment();
     if (PrevToken == NULL)
       return TT_UnaryOperator;
     if (PrevToken->isOneOf(tok::r_paren, tok::r_square, tok::identifier))
@@ -784,7 +773,7 @@ public:
       return;
 
     // Eagerly consume trailing comments.
-    while (isTrailingComment(Current)) {
+    while (Current && Current->isTrailingComment()) {
       next();
     }
 
@@ -807,7 +796,7 @@ public:
 
       // At the end of the line or when an operator with higher precedence is
       // found, insert fake parenthesis and return.
-      if (Current == NULL || closesScope(*Current) ||
+      if (Current == NULL || Current->closesScope() ||
           (CurrentPrecedence != 0 && CurrentPrecedence < Precedence)) {
         if (OperatorFound) {
           Start->FakeLParens.push_back(prec::Level(Precedence - 1));
@@ -818,8 +807,8 @@ public:
       }
 
       // Consume scopes: (), [], <> and {}
-      if (opensScope(*Current)) {
-        while (Current && !closesScope(*Current)) {
+      if (Current->opensScope()) {
+        while (Current && !Current->closesScope()) {
           next();
           parse();
         }
@@ -881,7 +870,7 @@ void TokenAnnotator::calculateFormatting
       Current->MustBreakBefore = true;
     } else if (Current->Type == TT_LineComment) {
       Current->MustBreakBefore = Current->FormatTok.NewlinesBefore > 0;
-    } else if (isTrailingComment(Current->Parent) ||
+    } else if (Current->Parent->isTrailingComment() ||
                (Current->is(tok::string_literal) &&
                 Current->Parent->is(tok::string_literal))) {
       Current->MustBreakBefore = true;
@@ -964,7 +953,7 @@ unsigned TokenAnnotator::splitPenalty(co
   if (Left.is(tok::colon) && Left.Type == TT_ObjCMethodExpr)
     return 20;
 
-  if (opensScope(Left))
+  if (Left.opensScope())
     return Left.ParameterCount > 1 ? prec::Comma : 20;
 
   if (Right.is(tok::lessless)) {
@@ -1083,7 +1072,7 @@ bool TokenAnnotator::spaceRequiredBefore
     return false;
   if (Tok.is(tok::colon))
     return !Line.First.isOneOf(tok::kw_case, tok::kw_default) &&
-           !Tok.Children.empty() && Tok.Type != TT_ObjCMethodExpr;
+           Tok.getNextNoneComment() != NULL && Tok.Type != TT_ObjCMethodExpr;
   if (Tok.is(tok::l_paren) && !Tok.Children.empty() &&
       Tok.Children[0].Type == TT_PointerOrReference &&
       !Tok.Children[0].Children.empty() &&
@@ -1170,7 +1159,7 @@ bool TokenAnnotator::canBreakBefore(cons
     return false;
   if (Left.is(tok::identifier) && Right.is(tok::string_literal))
     return true;
-  return (isBinaryOperator(Left) && Left.isNot(tok::lessless)) ||
+  return (Left.isBinaryOperator() && Left.isNot(tok::lessless)) ||
          Left.isOneOf(tok::comma, tok::coloncolon, tok::semi, tok::l_brace,
                       tok::kw_class, tok::kw_struct) ||
          Right.isOneOf(tok::lessless, tok::arrow, tok::period, tok::colon) ||

Modified: cfe/trunk/lib/Format/TokenAnnotator.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.h?rev=179167&r1=179166&r2=179167&view=diff
==============================================================================
--- cfe/trunk/lib/Format/TokenAnnotator.h (original)
+++ cfe/trunk/lib/Format/TokenAnnotator.h Wed Apr 10 04:49:49 2013
@@ -121,6 +121,15 @@ public:
             Children[0].isObjCAtKeyword(tok::objc_private));
   }
 
+  /// \brief Returns whether \p Tok is ([{ or a template opening <.
+  bool opensScope() const;
+  /// \brief Returns whether \p Tok is )]} or a template opening >.
+  bool closesScope() const;
+
+  bool isUnaryOperator() const;
+  bool isBinaryOperator() const;
+  bool isTrailingComment() const;
+
   FormatToken FormatTok;
 
   TokenType Type;
@@ -175,12 +184,11 @@ public:
   /// Only set if \c Type == \c TT_StartOfName.
   bool PartOfMultiVariableDeclStmt;
 
-  const AnnotatedToken *getPreviousNoneComment() const {
-    AnnotatedToken *Tok = Parent;
-    while (Tok != NULL && Tok->is(tok::comment))
-      Tok = Tok->Parent;
-    return Tok;
-  }
+  /// \brief Returns the previous token ignoring comments.
+  AnnotatedToken *getPreviousNoneComment() const;
+
+  /// \brief Returns the next token ignoring comments.
+  const AnnotatedToken *getNextNoneComment() const;
 };
 
 class AnnotatedLine {
@@ -227,11 +235,6 @@ inline prec::Level getPrecedence(const A
   return getBinOpPrecedence(Tok.FormatTok.Tok.getKind(), true, true);
 }
 
-/// \brief Returns whether \p Tok is ([{ or a template opening <.
-bool opensScope(const AnnotatedToken &Tok);
-/// \brief Returns whether \p Tok is )]} or a template opening >.
-bool closesScope(const AnnotatedToken &Tok);
-
 /// \brief Determines extra information about the tokens comprising an
 /// \c UnwrappedLine.
 class TokenAnnotator {

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=179167&r1=179166&r2=179167&view=diff
==============================================================================
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Wed Apr 10 04:49:49 2013
@@ -954,6 +954,7 @@ TEST_F(FormatTest, DoesNotBreakSemiAfter
 TEST_F(FormatTest, UnderstandsAccessSpecifiers) {
   verifyFormat("class A {\n"
                "public:\n"
+               "public: // comment\n"
                "protected:\n"
                "private:\n"
                "  void f() {}\n"





More information about the cfe-commits mailing list