r273694 - clang-format: [JS] Fix build breakage.

[Martin Probst via cfe-commits]

I double checked and cannot reproduce, nor can I find the test breakage. I
think at the time the conditional-in-parameter-initializer test case failed
without the change. If all tests pass, more power to you :-)

Martin Probst <martin at probst.io> schrieb am Di., 12. Juli 2016 um 07:32 Uhr:

> What do you mean by other tests? On my machine, reverting this change
> breaks one of the tests in FormatTestJS.Conditional.
>
> Martin
>
> Daniel Jasper <djasper at google.com> schrieb am Di., 12. Juli 2016 um
> 06:03 Uhr:
>
>> So, turns out top-level conditionals are actually used somewhat
>> frequently. Can I undo this change? It doesn't break any other tests. Do
>> you have other tests that I should run against?
>>
>> On Fri, Jun 24, 2016 at 11:48 PM, Martin Probst <martin at probst.io> wrote:
>>
>>> Yes, test breakage. The problem was that with the change fields and
>>> interfaces would still get incorrectly formatted (see also the comment on
>>> the diff). Will include it in the commit message next time.
>>>
>>>
>>> Daniel Jasper <djasper at google.com> schrieb am Fr., 24. Juni 2016 um
>>> 14:43 Uhr:
>>>
>>>> The patch description seems wrong as this doesn't fix a build breakage
>>>> AFAICT. Do you mean a test failure? If so, it would be helpful to #include
>>>> what's actually changing (before/after or calling out the failing test case
>>>> or something).
>>>>
>>>>
>>>> On Fri, Jun 24, 2016 at 7:45 PM, Martin Probst via cfe-commits <
>>>> cfe-commits at lists.llvm.org> wrote:
>>>>
>>>>> Author: mprobst
>>>>> Date: Fri Jun 24 12:45:13 2016
>>>>> New Revision: 273694
>>>>>
>>>>> URL: http://llvm.org/viewvc/llvm-project?rev=273694&view=rev
>>>>> Log:
>>>>> clang-format: [JS] Fix build breakage.
>>>>>
>>>>> Checking Line.MustBeDeclaration does actually break the field and
>>>>> param initializer use case.
>>>>>
>>>>> Modified:
>>>>>     cfe/trunk/lib/Format/TokenAnnotator.cpp
>>>>>     cfe/trunk/unittests/Format/FormatTestJS.cpp
>>>>>
>>>>> Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=273694&r1=273693&r2=273694&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)
>>>>> +++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Jun 24 12:45:13 2016
>>>>> @@ -639,7 +639,7 @@ private:
>>>>>        }
>>>>>        // Declarations cannot be conditional expressions, this can
>>>>> only be part
>>>>>        // of a type declaration.
>>>>> -      if (Line.MustBeDeclaration && !Contexts.back().IsExpression &&
>>>>> +      if (!Contexts.back().IsExpression &&
>>>>>            Style.Language == FormatStyle::LK_JavaScript)
>>>>>          break;
>>>>>        parseConditional();
>>>>>
>>>>> Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp
>>>>> URL:
>>>>> http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=273694&r1=273693&r2=273694&view=diff
>>>>>
>>>>> ==============================================================================
>>>>> --- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)
>>>>> +++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Jun 24 12:45:13
>>>>> 2016
>>>>> @@ -1351,7 +1351,7 @@ TEST_F(FormatTestJS, NonNullAssertionOpe
>>>>>
>>>>>  TEST_F(FormatTestJS, Conditional) {
>>>>>    verifyFormat("y = x ? 1 : 2;");
>>>>> -  verifyFormat("x ? 1 : 2;");
>>>>> +  verifyFormat("x ? 1: 2;"); // Known issue with top level
>>>>> conditionals.
>>>>>    verifyFormat("class Foo {\n"
>>>>>                 "  field = true ? 1 : 2;\n"
>>>>>                 "  method(a = true ? 1 : 2) {}\n"
>>>>>
>>>>>
>>>>> _______________________________________________
>>>>> cfe-commits mailing list
>>>>> cfe-commits at lists.llvm.org
>>>>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>>>>>
>>>>
>>>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160713/b100effe/attachment.html>