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

Jean-Philippe Dufraigne j.dufraigne at gmail.com
Mon Feb 23 13:37:10 PST 2015


Thank you very much for the feedback.
I already published an updated patch.
I reply inline to the questions you raised during the review.


================
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;
----------------
djasper wrote:
> Which of the new tests does this affect?
The ones with the && at the end of the operator:

  [ RUN      ] FormatTest.UnderstandsFunctionRefQualification
  Actual: "Deleted &operator=(const Deleted &) && = delete;"
  Expected: "Deleted &operator=(const Deleted &)&& = delete;"
  Actual: "SomeType MemberFunction(const Deleted &) && = delete;"
  Expected: "SomeType MemberFunction(const Deleted &)&& = delete;"
  [  FAILED  ] FormatTest.UnderstandsFunctionRefQualification (140 ms)  

================
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;
----------------
djasper wrote:
> I think this must have been there for a reason. What broke when you just removed this?
Removing this from master just break these tests:

  [ RUN      ] FormatTest.UnderstandsOverloadedOperators
  Actual: "Deleted &operator=(const Deleted &) & = default;"
  Expected: "Deleted &operator=(const Deleted &)& = default;"
  Actual: "Deleted &operator=(const Deleted &) && = delete;"
  Expected:"Deleted &operator=(const Deleted &)&& = delete;"
  Actual: "Deleted& operator=(const Deleted&) && = delete;"
  Expected:"Deleted& operator=(const Deleted&)&& = delete;"
  [  FAILED  ] FormatTest.UnderstandsOverloadedOperators (234 ms)
  [----------] Global test environment tear-down
  [==========] 315 tests from 4 test cases ran. (47166 ms total)
  [  PASSED  ] 314 tests.
  [  FAILED  ] 1 test, listed below:
  [  FAILED  ] FormatTest.UnderstandsOverloadedOperators

================
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() ||
----------------
djasper wrote:
> 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)"?
Ran with:
  if (Right.is(TT_PointerOrReference))
    return !Left.is(tok::r_paren) &&
           (Left.Tok.isLiteral() ||
            (!Left.isOneOf(TT_PointerOrReference, tok::l_paren) &&
             Style.PointerAlignment != FormatStyle::PAS_Left));

  [ RUN      ] FormatTest.UnderstandsUsesOfStarAndAmp
  Actual: "typedef typeof(int(int, int))*MyFunc;"
  Expected: "typedef typeof(int(int, int)) *MyFunc;"
  Actual: "[](const decltype(*a)&value) {}"
  Expected: "[](const decltype(*a) &value) {}"
  [  FAILED  ] FormatTest.UnderstandsUsesOfStarAndAmp (1451 ms)

http://reviews.llvm.org/D7813

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






More information about the cfe-commits mailing list