r183009 - Improve clang-format's error recovery.

[Manuel Klimek]

On Mon, Jun 3, 2013 at 6:11 PM, Daniel Jasper <djasper at google.com> wrote:

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

Yes, but currently we can also do it if the next token is not "}", or am I
missing the check?


>
>
> 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/56da8224/attachment.html>