From 4444917f5c5586a9283ea4c77e912d67cb75269c Mon Sep 17 00:00:00 2001 From: Nico Weber Date: Tue, 12 Feb 2013 16:17:07 +0000 Subject: [PATCH] Formatter: Correctly format stars in `sizeof(int**)` and similar places. 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(P); Now: return sizeof(int**); Type **A = static_cast(P); While here, also make all methods of AnnotatingParser except parseLine() private. Review URL: http://llvm-reviews.chandlerc.com/D384 llvm-svn: 174975 --- clang/lib/Format/TokenAnnotator.cpp | 34 ++++++++++++++++++++-------------- clang/unittests/Format/FormatTest.cpp | 11 +++++++++-- 2 files changed, 29 insertions(+), 16 deletions(-) diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp index 1e814b2..80089de 100644 --- a/clang/lib/Format/TokenAnnotator.cpp +++ b/clang/lib/Format/TokenAnnotator.cpp @@ -86,6 +86,7 @@ public: Contexts.back().LookForFunctionName = Line.MustBeDeclaration; } +private: bool parseAngle() { if (CurrentToken == NULL) return false; @@ -448,6 +449,7 @@ public: next(); } +public: LineType parseLine() { int PeriodsAndArrows = 0; bool CanBeBuilderTypeStmt = true; @@ -483,6 +485,7 @@ public: return LT_Other; } +private: void next() { if (CurrentToken != NULL) { determineTokenType(*CurrentToken); @@ -495,7 +498,6 @@ public: CurrentToken = NULL; } -private: /// \brief A struct to hold information valid in a specific context, e.g. /// a pair of parenthesis. struct Context { @@ -533,19 +535,25 @@ private: 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) || - (Current.is(tok::l_paren) && !Line.MustBeDeclaration && - (Current.Parent == NULL || Current.Parent->isNot(tok::kw_for)))) + } else if (Current.is(tok::kw_return) || Current.is(tok::kw_throw) || + (Current.is(tok::l_paren) && !Line.MustBeDeclaration && + (!Current.Parent || 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)) { @@ -639,10 +647,6 @@ private: 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) @@ -932,9 +936,11 @@ bool TokenAnnotator::spaceRequiredBetween(const AnnotatedLine &Line, 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)) diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index 9b9f5c7..e9520b6 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -1615,6 +1615,14 @@ TEST_F(FormatTest, UnderstandsUsesOfStarAndAmp) { 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(P);"); + verifyGoogleFormat("Type** A = static_cast(P);"); + // FIXME: The newline is wrong. + verifyFormat("auto a = [](int **&, int ***) {}\n;"); verifyIndependentOfContext("InvalidRegions[*R] = 0;"); @@ -2540,8 +2548,7 @@ TEST_F(FormatTest, ObjCSnippets) { 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" -- 2.7.4