[PATCH] Fix spacing for function with ref-qualification when using SpacesInCStyleCastParentheses != SpacesInParentheses

Daniel Jasper djasper at google.com
Mon Feb 23 04:30:00 PST 2015


================
Comment at: lib/Format/FormatToken.h:46
@@ -45,2 +45,3 @@
   TT_FunctionLBrace,
+  TT_FunctionLParen,
   TT_FunctionTypeLParen,
----------------
I am not convinced that we actually need a separate token type to fix this.

================
Comment at: lib/Format/TokenAnnotator.cpp:463
@@ -462,1 +462,3 @@
     case tok::l_paren:
+      if (Tok->Previous && Tok->Previous->is(tok::identifier))
+        Tok->Type = TT_FunctionLParen;
----------------
I think, this is wrong. "<identifier>(" can be different things.

================
Comment at: lib/Format/TokenAnnotator.cpp:771-772
@@ -768,4 +770,4 @@
           if (Previous->isOneOf(TT_BinaryOperator, TT_UnaryOperator) &&
-              Previous->isOneOf(tok::star, tok::amp) && Previous->Previous &&
-              Previous->Previous->isNot(tok::equal))
+              Previous->isOneOf(tok::star, tok::amp, tok::ampamp) &&
+              Previous->Previous && Previous->Previous->isNot(tok::equal))
             Previous->Type = TT_PointerOrReference;
----------------
Which of the new tests does this affect?

================
Comment at: lib/Format/TokenAnnotator.cpp:807
@@ -804,1 +806,3 @@
       Contexts.back().IsExpression = true;
+      if (Current.is(tok::semi) && Line.MustBeDeclaration && Current.Previous &&
+          Current.Previous->isOneOf(tok::star, tok::amp, tok::ampamp))
----------------
I think a */& before a semicolon is always a pointer/reference, no?

There is a test in determineStarAmpUsage:

  if (NextToken->isOneOf(tok::kw_operator, tok::comma))

I think we can just add semi to that list instead of this change.

================
Comment at: lib/Format/TokenAnnotator.cpp:980
@@ -974,4 +979,3 @@
     if (ParensAreType && !ParensCouldEndDecl && !IsSizeOfOrAlignOf &&
-        ((Contexts.size() > 1 && Contexts[Contexts.size() - 2].IsExpression) ||
-         (Tok.Next && Tok.Next->isBinaryOperator())))
+        (Contexts.size() > 1 && Contexts[Contexts.size() - 2].IsExpression))
       IsCast = true;
----------------
I think this must have been there for a reason. What broke when you just removed this?

================
Comment at: lib/Format/TokenAnnotator.cpp:1670-1672
@@ -1665,4 +1669,5 @@
   if (Right.is(TT_PointerOrReference))
-    return Left.Tok.isLiteral() ||
-           (!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
-            Style.PointerAlignment != FormatStyle::PAS_Left);
+    return !(Left.is(tok::r_paren) && Left.MatchingParen &&
+             Left.MatchingParen->isOneOf(TT_OverloadedOperatorLParen,
+                                         TT_FunctionLParen)) &&
+           (Left.Tok.isLiteral() ||
----------------
In what cases do we want a space between ")" and a pointer/references? The case where it is a TT_CastRParen is already handled at this point. So my question is, what happens if you remove everything of what you have added except "Left.is(tok::r_paren)"?

http://reviews.llvm.org/D7813

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the cfe-commits mailing list