<div dir="ltr"><div class="gmail_extra">Not necessarily. I don't care what the output in those particular cases is. I want them to not crash, not access uninitialized memory, etc.</div><div class="gmail_extra"><br></div><div class="gmail_extra">clang-format has certain logic that makes it do good modification to invalid code, which is highly useful. We aren't copping out of formatting code just because it is broken. However, in these particular cases, I don't care. Yes, I could try to come up with a saner test that both crashes clang-format as well as contains input where I care about the formatting, but I don't think that that would be effort well-spent.</div><div class="gmail_extra"><br><div class="gmail_quote">On Thu, Jan 29, 2015 at 8:52 PM, David Blaikie <span dir="ltr"><<a href="mailto:dblaikie@gmail.com" target="_blank" class="cremed">dblaikie@gmail.com</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"><br><div class="gmail_extra"><br><div class="gmail_quote"><div><div class="h5">On Thu, Jan 29, 2015 at 2:47 AM, Daniel Jasper <span dir="ltr"><<a href="mailto:djasper@google.com" target="_blank" class="cremed">djasper@google.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Author: djasper<br>
Date: Thu Jan 29 04:47:14 2015<br>
New Revision: 227427<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=227427&view=rev" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project?rev=227427&view=rev</a><br>
Log:<br>
clang-format: Fix crasher caused by not properly setting dry-run.<br>
<br>
Modified:<br>
cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp<br>
cfe/trunk/lib/Format/WhitespaceManager.cpp<br>
cfe/trunk/unittests/Format/FormatTest.cpp<br>
<br>
Modified: cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=227427&r1=227426&r2=227427&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp?rev=227427&r1=227426&r2=227427&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp (original)<br>
+++ cfe/trunk/lib/Format/UnwrappedLineFormatter.cpp Thu Jan 29 04:47:14 2015<br>
@@ -409,7 +409,7 @@ UnwrappedLineFormatter::format(const Sma<br>
TheLine.Type == LT_ImportStatement) {<br>
LineState State = Indenter->getInitialState(Indent, &TheLine, DryRun);<br>
while (State.NextToken) {<br>
- formatChildren(State, /*Newline=*/false, /*DryRun=*/false, Penalty);<br>
+ formatChildren(State, /*Newline=*/false, DryRun, Penalty);<br>
Indenter->addTokenToState(State, /*Newline=*/false, DryRun);<br>
}<br>
} else if (Style.ColumnLimit == 0) {<br>
@@ -657,8 +657,8 @@ void UnwrappedLineFormatter::addNextStat<br>
<br>
bool UnwrappedLineFormatter::formatChildren(LineState &State, bool NewLine,<br>
bool DryRun, unsigned &Penalty) {<br>
- FormatToken &Previous = *State.NextToken->Previous;<br>
const FormatToken *LBrace = State.NextToken->getPreviousNonComment();<br>
+ FormatToken &Previous = *State.NextToken->Previous;<br>
if (!LBrace || LBrace->isNot(tok::l_brace) || LBrace->BlockKind != BK_Block ||<br>
Previous.Children.size() == 0)<br>
// The previous token does not open a block. Nothing to do. We don't<br>
<br>
Modified: cfe/trunk/lib/Format/WhitespaceManager.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=227427&r1=227426&r2=227427&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/WhitespaceManager.cpp?rev=227427&r1=227426&r2=227427&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/lib/Format/WhitespaceManager.cpp (original)<br>
+++ cfe/trunk/lib/Format/WhitespaceManager.cpp Thu Jan 29 04:47:14 2015<br>
@@ -264,6 +264,11 @@ void WhitespaceManager::alignEscapedNewl<br>
void WhitespaceManager::generateChanges() {<br>
for (unsigned i = 0, e = Changes.size(); i != e; ++i) {<br>
const Change &C = Changes[i];<br>
+ if (i > 0) {<br>
+ assert(Changes[i - 1].OriginalWhitespaceRange.getBegin() !=<br>
+ C.OriginalWhitespaceRange.getBegin() &&<br>
+ "Generating two replacements for the same location");<br>
+ }<br>
if (C.CreateReplacement) {<br>
std::string ReplacementText = C.PreviousLinePostfix;<br>
if (C.ContinuesPPDirective)<br>
<br>
Modified: cfe/trunk/unittests/Format/FormatTest.cpp<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=227427&r1=227426&r2=227427&view=diff" target="_blank" class="cremed">http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=227427&r1=227426&r2=227427&view=diff</a><br>
==============================================================================<br>
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)<br>
+++ cfe/trunk/unittests/Format/FormatTest.cpp Thu Jan 29 04:47:14 2015<br>
@@ -3081,6 +3081,8 @@ TEST_F(FormatTest, LayoutNestedBlocks) {<br>
" if (aaaaaaaaaaaaaaaaaaaaaaaa) return;\n"<br>
"}, a);",<br>
Style);<br>
+<br>
+ verifyNoCrash("^{v^{a}}");<br></blockquote></div></div><div><br>"not crashing" is a pretty low bar for a test. Is there some behavior that should be expected? (I guess most of these crashers are crash-on-invalid and the correct behavior is to not modify the text?)<br> </div><span class=""><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
}<br>
<br>
TEST_F(FormatTest, IndividualStatementsOfNestedBlocks) {<br>
<br>
<br>
_______________________________________________<br>
cfe-commits mailing list<br>
<a href="mailto:cfe-commits@cs.uiuc.edu" target="_blank" class="cremed">cfe-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits" target="_blank" class="cremed">http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits</a><br>
</blockquote></span></div><br></div></div>
</blockquote></div><br></div></div>