r183009 - Improve clang-format's error recovery.

[Manuel Klimek]

On Fri, May 31, 2013 at 4:56 PM, Daniel Jasper <djasper at google.com> wrote:

> Author: djasper
> Date: Fri May 31 09:56:29 2013
> New Revision: 183009
>
> URL: http://llvm.org/viewvc/llvm-project?rev=183009&view=rev
> Log:
> Improve clang-format's error recovery.
>
> If a "}" is found inside parenthesis, this is probably a case of
> missing parenthesis. This enables continuing to format after stuff code
> like:
>
> class A {
>   void f(
> };
> ..
>
> Modified:
>     cfe/trunk/lib/Format/UnwrappedLineParser.cpp
>     cfe/trunk/unittests/Format/FormatTest.cpp
>
> Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=183009&r1=183008&r2=183009&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
> +++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Fri May 31 09:56:29 2013
> @@ -45,6 +45,7 @@ public:
>      else
>        Line.MustBeDeclaration = true;
>    }
> +
>  private:
>    UnwrappedLine &Line;
>    std::vector<bool> &Stack;
> @@ -81,9 +82,7 @@ public:
>      return Token;
>    }
>
> -  virtual unsigned getPosition() {
> -    return PreviousTokenSource->getPosition();
> -  }
> +  virtual unsigned getPosition() { return
> PreviousTokenSource->getPosition(); }
>
>    virtual FormatToken *setPosition(unsigned Position) {
>      Token = PreviousTokenSource->setPosition(Position);
> @@ -279,7 +278,7 @@ void UnwrappedLineParser::calculateBrace
>      case tok::kw_for:
>      case tok::kw_switch:
>      case tok::kw_try:
> -      if (!LBraceStack.empty())
> +      if (!LBraceStack.empty())
>          LBraces[LBraceStack.back()] = BS_Block;
>        break;
>      default:
> @@ -386,9 +385,7 @@ void UnwrappedLineParser::parsePPElse()
>    parsePPUnknown();
>  }
>
> -void UnwrappedLineParser::parsePPElIf() {
> -  parsePPElse();
> -}
> +void UnwrappedLineParser::parsePPElIf() { parsePPElse(); }
>
>  void UnwrappedLineParser::parsePPEndIf() {
>    if (!PPStack.empty())
> @@ -700,15 +697,21 @@ void UnwrappedLineParser::parseParens()
>      case tok::r_paren:
>        nextToken();
>        return;
> +    case tok::r_brace:
> +      // A "}" inside parenthesis is an error if there wasn't a matching
> "{".
> +      return;
>      case tok::l_brace: {
>        if (!tryToParseBracedList()) {
>          nextToken();
> -        ScopedLineState LineState(*this);
> -        ScopedDeclarationState DeclarationState(*Line,
> DeclarationScopeStack,
> -                                                /*MustBeDeclaration=*/
> false);
> -        Line->Level += 1;
> -        parseLevel(/*HasOpeningBrace=*/ true);
> -        Line->Level -= 1;
> +        {
> +          ScopedLineState LineState(*this);
> +          ScopedDeclarationState DeclarationState(*Line,
> DeclarationScopeStack,
> +                                                  /*MustBeDeclaration=*/
> false);
> +          Line->Level += 1;
> +          parseLevel(/*HasOpeningBrace=*/ true);
> +          Line->Level -= 1;
> +        }
> +        nextToken();
>

Don't we only want to do this if the next token is a '{'?


>        }
>        break;
>      }
>
> Modified: cfe/trunk/unittests/Format/FormatTest.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=183009&r1=183008&r2=183009&view=diff
>
> ==============================================================================
> --- cfe/trunk/unittests/Format/FormatTest.cpp (original)
> +++ cfe/trunk/unittests/Format/FormatTest.cpp Fri May 31 09:56:29 2013
> @@ -1901,6 +1901,19 @@ TEST_F(FormatTest, LayoutBlockInsidePare
>              "  int i;\n"
>              "});",
>              format(" functionCall ( {int i;} );"));
> +
> +  // FIXME: This is bad, find a better and more generic solution.
> +  EXPECT_EQ("functionCall({\n"
> +            "  int i;\n"
> +            "},\n"
> +            "             aaaa, bbbb, cccc);",
> +            format(" functionCall ( {int i;},  aaaa,   bbbb, cccc);"));
> +  verifyFormat(
> +      "Aaa({\n"
> +      "  int i;\n"
> +      "},\n"
> +      "
>  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,\n"
> +      "                                     ccccccccccccccccc));");
>  }
>
>  TEST_F(FormatTest, LayoutBlockInsideStatement) {
> @@ -3387,7 +3400,9 @@ TEST_F(FormatTest, IncorrectCodeMissingP
>
>  TEST_F(FormatTest, DoesNotTouchUnwrappedLinesWithErrors) {
>    verifyFormat("namespace {\n"
> -               "class Foo {  Foo  ( }; }  // comment");
> +               "class Foo {  Foo  (\n"
> +               "};\n"
> +               "} // comment");
>  }
>
>  TEST_F(FormatTest, IncorrectCodeErrorDetection) {
> @@ -3460,16 +3475,6 @@ TEST_F(FormatTest, LayoutCxx11Constructo
>                   NoSpaces);
>  }
>
> -TEST_F(FormatTest, LayoutTokensFollowingBlockInParentheses) {
> -  // FIXME: This is bad, find a better and more generic solution.
> -  verifyFormat(
> -      "Aaa({\n"
> -      "  int i;\n"
> -      "},\n"
> -      "
>  aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb,\n"
> -      "                                     ccccccccccccccccc));");
> -}
> -
>  TEST_F(FormatTest, PullTrivialFunctionDefinitionsIntoSingleLine) {
>    verifyFormat("void f() { return 42; }");
>    verifyFormat("void f() {\n"
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20130603/2157e5c1/attachment.html>