<div dir="ltr">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 :-)<div><br><div class="gmail_quote"><div dir="ltr">Martin Probst <<a href="mailto:martin@probst.io" target="_blank">martin@probst.io</a>> schrieb am Di., 12. Juli 2016 um 07:32 Uhr:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">What do you mean by other tests? On my machine, reverting this change breaks one of the tests in FormatTestJS.Conditional.</div><div dir="ltr"><div><br></div><div>Martin</div></div><br><div class="gmail_quote"><div dir="ltr">Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>> schrieb am Di., 12. Juli 2016 um 06:03 Uhr:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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?</div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 24, 2016 at 11:48 PM, Martin Probst <span dir="ltr"><<a href="mailto:martin@probst.io" target="_blank">martin@probst.io</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.<div><div class="m_-8608805340982609451m_7801748325714710127m_9169540555311715349h5"><br><br><div class="gmail_quote"><div dir="ltr">Daniel Jasper <<a href="mailto:djasper@google.com" target="_blank">djasper@google.com</a>> schrieb am Fr., 24. Juni 2016 um 14:43 Uhr:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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).<div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Fri, Jun 24, 2016 at 7:45 PM, Martin Probst via cfe-commits <span dir="ltr"><<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: mprobst<br>
Date: Fri Jun 24 12:45:13 2016<br>
New Revision: 273694<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=273694&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=273694&view=rev</a><br>
Log:<br>
clang-format: [JS] Fix build breakage.<br>
<br>
Checking Line.MustBeDeclaration does actually break the field and param initializer use case.<br>
<br>
Modified:<br>
cfe/trunk/lib/Format/TokenAnnotator.cpp<br>
cfe/trunk/unittests/Format/FormatTestJS.cpp<br>
<br>
Modified: cfe/trunk/lib/Format/TokenAnnotator.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=273694&r1=273693&r2=273694&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/TokenAnnotator.cpp?rev=273694&r1=273693&r2=273694&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/TokenAnnotator.cpp (original)<br>
+++ cfe/trunk/lib/Format/TokenAnnotator.cpp Fri Jun 24 12:45:13 2016<br>
@@ -639,7 +639,7 @@ private:<br>
}<br>
// Declarations cannot be conditional expressions, this can only be part<br>
// of a type declaration.<br>
- if (Line.MustBeDeclaration && !Contexts.back().IsExpression &&<br>
+ if (!Contexts.back().IsExpression &&<br>
Style.Language == FormatStyle::LK_JavaScript)<br>
break;<br>
parseConditional();<br>
<br>
Modified: cfe/trunk/unittests/Format/FormatTestJS.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=273694&r1=273693&r2=273694&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTestJS.cpp?rev=273694&r1=273693&r2=273694&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTestJS.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTestJS.cpp Fri Jun 24 12:45:13 2016<br>
@@ -1351,7 +1351,7 @@ TEST_F(FormatTestJS, NonNullAssertionOpe<br>
<br>
TEST_F(FormatTestJS, Conditional) {<br>
verifyFormat("y = x ? 1 : 2;");<br>
- verifyFormat("x ? 1 : 2;");<br>
+ verifyFormat("x ? 1: 2;"); // Known issue with top level conditionals.<br>
verifyFormat("class Foo {\n"<br>
" field = true ? 1 : 2;\n"<br>
" method(a = true ? 1 : 2) {}\n"<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@lists.llvm.org" target="_blank">cfe-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits</a><br>
</blockquote></div><br></div>
</blockquote></div></div></div></div>
</blockquote></div><br></div>
</blockquote></div></blockquote></div></div></div>