From 70b03f4edaefcc5b9aa2e084d1c12e9d91b32a77 Mon Sep 17 00:00:00 2001 From: Manuel Klimek Date: Wed, 23 Jan 2013 09:32:48 +0000 Subject: [PATCH] Allow us to better guess the context of an unwrapped line. This gives us the ability to guess better defaults for whether a * between identifiers is a pointer dereference or binary operator. Now correctly formats: void f(a *b); void f() { f(a * b); } git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@173243 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/Format/Format.cpp | 9 +++-- lib/Format/UnwrappedLineParser.cpp | 53 ++++++++++++++++++++++-------- lib/Format/UnwrappedLineParser.h | 10 ++++-- unittests/Format/FormatTest.cpp | 5 +++ 4 files changed, 58 insertions(+), 19 deletions(-) diff --git a/lib/Format/Format.cpp b/lib/Format/Format.cpp index 1405fc23e1..391528a031 100644 --- a/lib/Format/Format.cpp +++ b/lib/Format/Format.cpp @@ -112,7 +112,8 @@ class AnnotatedLine { public: AnnotatedLine(const UnwrappedLine &Line) : First(Line.Tokens.front()), Level(Line.Level), - InPPDirective(Line.InPPDirective) { + InPPDirective(Line.InPPDirective), + MustBeDeclaration(Line.MustBeDeclaration) { assert(!Line.Tokens.empty()); AnnotatedToken *Current = &First; for (std::list::const_iterator I = ++Line.Tokens.begin(), @@ -126,7 +127,8 @@ public: } AnnotatedLine(const AnnotatedLine &Other) : First(Other.First), Type(Other.Type), Level(Other.Level), - InPPDirective(Other.InPPDirective) { + InPPDirective(Other.InPPDirective), + MustBeDeclaration(Other.MustBeDeclaration) { Last = &First; while (!Last->Children.empty()) { Last->Children[0].Parent = Last; @@ -140,6 +142,7 @@ public: LineType Type; unsigned Level; bool InPPDirective; + bool MustBeDeclaration; }; static prec::Level getPrecedence(const AnnotatedToken &Tok) { @@ -1368,7 +1371,7 @@ private: // It is very unlikely that we are going to find a pointer or reference type // definition on the RHS of an assignment. - if (IsRHS) + if (IsRHS || !Line.MustBeDeclaration) return TT_BinaryOperator; return TT_PointerOrReference; diff --git a/lib/Format/UnwrappedLineParser.cpp b/lib/Format/UnwrappedLineParser.cpp index 53fa2a32df..047074135c 100644 --- a/lib/Format/UnwrappedLineParser.cpp +++ b/lib/Format/UnwrappedLineParser.cpp @@ -28,6 +28,23 @@ namespace clang { namespace format { +class ScopedDeclarationState { +public: + ScopedDeclarationState(UnwrappedLine &Line, std::vector &Stack, + bool MustBeDeclaration) + : Line(Line), Stack(Stack) { + Stack.push_back(MustBeDeclaration); + Line.MustBeDeclaration = MustBeDeclaration; + } + ~ScopedDeclarationState() { + Line.MustBeDeclaration = Stack.back(); + Stack.pop_back(); + } +private: + UnwrappedLine &Line; + std::vector &Stack; +}; + class ScopedMacroState : public FormatTokenSource { public: ScopedMacroState(UnwrappedLine &Line, FormatTokenSource *&TokenSource, @@ -128,6 +145,8 @@ bool UnwrappedLineParser::parse() { } bool UnwrappedLineParser::parseFile() { + ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack, + /*MustBeDeclaration=*/ true); bool Error = parseLevel(/*HasOpeningBrace=*/false); // Make sure to format the remaining tokens. flushComments(true); @@ -144,7 +163,9 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) { addUnwrappedLine(); break; case tok::l_brace: - Error |= parseBlock(); + // FIXME: Add parameter whether this can happen - if this happens, we must + // be in a non-declaration context. + Error |= parseBlock(/*MustBeDeclaration=*/ false); addUnwrappedLine(); break; case tok::r_brace: @@ -167,12 +188,14 @@ bool UnwrappedLineParser::parseLevel(bool HasOpeningBrace) { return Error; } -bool UnwrappedLineParser::parseBlock(unsigned AddLevels) { +bool UnwrappedLineParser::parseBlock(bool MustBeDeclaration, unsigned AddLevels) { assert(FormatTok.Tok.is(tok::l_brace) && "'{' expected"); nextToken(); addUnwrappedLine(); + ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack, + MustBeDeclaration); Line->Level += AddLevels; parseLevel(/*HasOpeningBrace=*/true); @@ -307,7 +330,7 @@ void UnwrappedLineParser::parseStructuralElement() { if (FormatTok.Tok.is(tok::string_literal)) { nextToken(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(0); + parseBlock(/*MustBeDeclaration=*/ true, 0); addUnwrappedLine(); return; } @@ -345,7 +368,7 @@ void UnwrappedLineParser::parseStructuralElement() { // structural element. // FIXME: Figure out cases where this is not true, and add projections for // them (the one we know is missing are lambdas). - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ false); addUnwrappedLine(); return; case tok::identifier: @@ -427,8 +450,10 @@ void UnwrappedLineParser::parseParens() { { nextToken(); ScopedLineState LineState(*this); + ScopedDeclarationState DeclarationState(*Line, DeclarationScopeStack, + /*MustBeDeclaration=*/ false); Line->Level += 1; - parseLevel(/*HasOpeningBrace=*/true); + parseLevel(/*HasOpeningBrace=*/ true); Line->Level -= 1; } break; @@ -446,7 +471,7 @@ void UnwrappedLineParser::parseIfThenElse() { parseParens(); bool NeedsUnwrappedLine = false; if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ false); NeedsUnwrappedLine = true; } else { addUnwrappedLine(); @@ -457,7 +482,7 @@ void UnwrappedLineParser::parseIfThenElse() { if (FormatTok.Tok.is(tok::kw_else)) { nextToken(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ false); addUnwrappedLine(); } else if (FormatTok.Tok.is(tok::kw_if)) { parseIfThenElse(); @@ -478,7 +503,7 @@ void UnwrappedLineParser::parseNamespace() { if (FormatTok.Tok.is(tok::identifier)) nextToken(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(0); + parseBlock(/*MustBeDeclaration=*/ true, 0); addUnwrappedLine(); } // FIXME: Add error handling. @@ -491,7 +516,7 @@ void UnwrappedLineParser::parseForOrWhileLoop() { if (FormatTok.Tok.is(tok::l_paren)) parseParens(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ false); addUnwrappedLine(); } else { addUnwrappedLine(); @@ -505,7 +530,7 @@ void UnwrappedLineParser::parseDoWhile() { assert(FormatTok.Tok.is(tok::kw_do) && "'do' expected"); nextToken(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ false); } else { addUnwrappedLine(); ++Line->Level; @@ -531,7 +556,7 @@ void UnwrappedLineParser::parseLabel() { if (Line->Level > 0) --Line->Level; if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ false); if (FormatTok.Tok.is(tok::kw_break)) parseStructuralElement(); // "break;" after "}" goes on the same line. } @@ -554,7 +579,7 @@ void UnwrappedLineParser::parseSwitch() { if (FormatTok.Tok.is(tok::l_paren)) parseParens(); if (FormatTok.Tok.is(tok::l_brace)) { - parseBlock(Style.IndentCaseLabels ? 2 : 1); + parseBlock(/*MustBeDeclaration=*/ false, Style.IndentCaseLabels ? 2 : 1); addUnwrappedLine(); } else { addUnwrappedLine(); @@ -648,7 +673,7 @@ void UnwrappedLineParser::parseRecord() { } } if (FormatTok.Tok.is(tok::l_brace)) - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ true); // We fall through to parsing a structural element afterwards, so // class A {} n, m; // will end up in one unwrapped line. @@ -690,7 +715,7 @@ void UnwrappedLineParser::parseObjCInterfaceOrImplementation() { // If instance variables are present, keep the '{' on the first line too. if (FormatTok.Tok.is(tok::l_brace)) - parseBlock(); + parseBlock(/*MustBeDeclaration=*/ true); // With instance variables, this puts '}' on its own line. Without instance // variables, this ends the @interface line. diff --git a/lib/Format/UnwrappedLineParser.h b/lib/Format/UnwrappedLineParser.h index f48594d592..1ab1ca21dc 100644 --- a/lib/Format/UnwrappedLineParser.h +++ b/lib/Format/UnwrappedLineParser.h @@ -84,7 +84,7 @@ struct FormatToken { /// \c UnwrappedLineFormatter. The key property is that changing the formatting /// within an unwrapped line does not affect any other unwrapped lines. struct UnwrappedLine { - UnwrappedLine() : Level(0), InPPDirective(false) { + UnwrappedLine() : Level(0), InPPDirective(false), MustBeDeclaration(false) { } // FIXME: Don't use std::list here. @@ -96,6 +96,8 @@ struct UnwrappedLine { /// \brief Whether this \c UnwrappedLine is part of a preprocessor directive. bool InPPDirective; + + bool MustBeDeclaration; }; class UnwrappedLineConsumer { @@ -124,7 +126,7 @@ public: private: bool parseFile(); bool parseLevel(bool HasOpeningBrace); - bool parseBlock(unsigned AddLevels = 1); + bool parseBlock(bool MustBeDeclaration, unsigned AddLevels = 1); void parsePPDirective(); void parsePPDefine(); void parsePPUnknown(); @@ -180,6 +182,10 @@ private: // \c &PreprocessorDirectives. std::vector *CurrentLines; + // We store for each line whether it must be a declaration depending on + // whether we are in a compound statement or not. + std::vector DeclarationScopeStack; + clang::DiagnosticsEngine &Diag; const FormatStyle &Style; FormatTokenSource *Tokens; diff --git a/unittests/Format/FormatTest.cpp b/unittests/Format/FormatTest.cpp index 3065a836dd..7daf7aac6c 100644 --- a/unittests/Format/FormatTest.cpp +++ b/unittests/Format/FormatTest.cpp @@ -1666,6 +1666,11 @@ TEST_F(FormatTest, BlockComments) { "/* */someCall(parameter);", getLLVMStyleWithColumns(15))); } +TEST_F(FormatTest, Fuck) { + verifyFormat("void f(int *a);"); + verifyFormat("void f() { f(fint * b); }"); +} + //===----------------------------------------------------------------------===// // Objective-C tests. //===----------------------------------------------------------------------===// -- 2.40.0