r183009 - Improve clang-format's error recovery.

[Daniel Jasper]

Ah, then probably: "no". But we have to decide carefully for each token. I
think it is pretty safe for "}".


On Mon, Jun 3, 2013 at 6:06 PM, Manuel Klimek <klimek at google.com> wrote:

> Sorry, meant '}'.
>
>
> On Mon, Jun 3, 2013 at 5:41 PM, Daniel Jasper <djasper at google.com> wrote:
>
>> I don't understand that. '{' would not be an error ..
>>
>>
>> On Mon, Jun 3, 2013 at 1:34 PM, Manuel Klimek <klimek at google.com> wrote:
>>
>>> 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/dc006351/attachment.html>