I'm not 100% certain that this is the commit which caused the breakage I am seeing, but operator< causes assertions to fail in MSVC's predicate debuggers. Long story short, for some LineStates, !pred(left, right) fails, and pred(right, left) succeeds which causes an assertion.
One thing which struck me as potentially odd was that the code was using Other > this in some places, and Other < this in other places. When I flip the logic to be consistent, all of the assertions quieted down. I've attached a patch showing the changes I've made, but I'd like some oversight to make sure I've not mucked things up. Thanks! ~Aaron On Mon, Feb 18, 2013 at 6:05 AM, Daniel Jasper <[email protected]> wrote: > Author: djasper > Date: Mon Feb 18 05:05:07 2013 > New Revision: 175432 > > URL: http://llvm.org/viewvc/llvm-project?rev=175432&view=rev > Log: > Prevent line breaks that make stuff hard to read. > > Before: > aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaa( > aaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaaaa( > aaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); > > After: > aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa) > .aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa) > .aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa, > aaaaaaaaaaaaaaaaaaa, > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); > > Modified: > cfe/trunk/lib/Format/Format.cpp > cfe/trunk/unittests/Format/FormatTest.cpp > > Modified: cfe/trunk/lib/Format/Format.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=175432&r1=175431&r2=175432&view=diff > ============================================================================== > --- cfe/trunk/lib/Format/Format.cpp (original) > +++ cfe/trunk/lib/Format/Format.cpp Mon Feb 18 05:05:07 2013 > @@ -251,6 +251,7 @@ public: > State.VariablePos = 0; > State.LineContainsContinuedForLoopSection = false; > State.ParenLevel = 0; > + State.StartOfLineLevel = State.ParenLevel; > > DEBUG({ > DebugTokenState(*State.NextToken); > @@ -384,6 +385,9 @@ private: > /// \brief The level of nesting inside (), [], <> and {}. > unsigned ParenLevel; > > + /// \brief The \c ParenLevel at the start of this line. > + unsigned StartOfLineLevel; > + > /// \brief A stack keeping track of properties applying to parenthesis > /// levels. > std::vector<ParenState> Stack; > @@ -401,6 +405,8 @@ private: > return LineContainsContinuedForLoopSection; > if (Other.ParenLevel != ParenLevel) > return Other.ParenLevel < ParenLevel; > + if (Other.StartOfLineLevel < StartOfLineLevel) > + return Other.StartOfLineLevel < StartOfLineLevel; > return Other.Stack < Stack; > } > }; > @@ -489,6 +495,7 @@ private: > } > > State.Stack.back().LastSpace = State.Column; > + State.StartOfLineLevel = State.ParenLevel; > if (Current.is(tok::colon) && Current.Type != TT_ConditionalExpr) > State.Stack.back().Indent += 2; > } else { > @@ -772,6 +779,14 @@ private: > !(State.NextToken->is(tok::r_brace) && > State.Stack.back().BreakBeforeClosingBrace)) > return false; > + // This prevents breaks like: > + // ... > + // SomeParameter, OtherParameter).DoSomething( > + // ... > + // As they hide "DoSomething" and generally bad for readability. > + if (State.NextToken->Parent->is(tok::l_paren) && > + State.ParenLevel <= State.StartOfLineLevel) > + return false; > // Trying to insert a parameter on a new line if there are already more > than > // one parameter on the current line is bin packing. > if (State.Stack.back().HasMultiParameterLine && > > Modified: cfe/trunk/unittests/Format/FormatTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=175432&r1=175431&r2=175432&view=diff > ============================================================================== > --- cfe/trunk/unittests/Format/FormatTest.cpp (original) > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Feb 18 05:05:07 2013 > @@ -1457,6 +1457,13 @@ TEST_F(FormatTest, WrapsAtFunctionCallsI > verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]\n" > " .insert(ccccccccccccccccccccccc);"); > > + verifyGoogleFormat( > + "aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n" > + " .aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n" > + " .aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa,\n" > + " aaaaaaaaaaaaaaaaaaa,\n" > + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); > + > // Here, it is not necessary to wrap at "." or "->". > verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) > ||\n" > " aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n}"); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits On Mon, Feb 18, 2013 at 6:05 AM, Daniel Jasper <[email protected]> wrote: > Author: djasper > Date: Mon Feb 18 05:05:07 2013 > New Revision: 175432 > > URL: http://llvm.org/viewvc/llvm-project?rev=175432&view=rev > Log: > Prevent line breaks that make stuff hard to read. > > Before: > aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaa( > aaaaaaaaaaaaaaaaaaaaa).aaaaaaaaaaaaaaaaaaa( > aaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaa, aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); > > After: > aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa) > .aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa) > .aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa, > aaaaaaaaaaaaaaaaaaa, > aaaaaaaaaaaaaaaaaaaaaaaaaaaaaa); > > Modified: > cfe/trunk/lib/Format/Format.cpp > cfe/trunk/unittests/Format/FormatTest.cpp > > Modified: cfe/trunk/lib/Format/Format.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/Format.cpp?rev=175432&r1=175431&r2=175432&view=diff > ============================================================================== > --- cfe/trunk/lib/Format/Format.cpp (original) > +++ cfe/trunk/lib/Format/Format.cpp Mon Feb 18 05:05:07 2013 > @@ -251,6 +251,7 @@ public: > State.VariablePos = 0; > State.LineContainsContinuedForLoopSection = false; > State.ParenLevel = 0; > + State.StartOfLineLevel = State.ParenLevel; > > DEBUG({ > DebugTokenState(*State.NextToken); > @@ -384,6 +385,9 @@ private: > /// \brief The level of nesting inside (), [], <> and {}. > unsigned ParenLevel; > > + /// \brief The \c ParenLevel at the start of this line. > + unsigned StartOfLineLevel; > + > /// \brief A stack keeping track of properties applying to parenthesis > /// levels. > std::vector<ParenState> Stack; > @@ -401,6 +405,8 @@ private: > return LineContainsContinuedForLoopSection; > if (Other.ParenLevel != ParenLevel) > return Other.ParenLevel < ParenLevel; > + if (Other.StartOfLineLevel < StartOfLineLevel) > + return Other.StartOfLineLevel < StartOfLineLevel; > return Other.Stack < Stack; > } > }; > @@ -489,6 +495,7 @@ private: > } > > State.Stack.back().LastSpace = State.Column; > + State.StartOfLineLevel = State.ParenLevel; > if (Current.is(tok::colon) && Current.Type != TT_ConditionalExpr) > State.Stack.back().Indent += 2; > } else { > @@ -772,6 +779,14 @@ private: > !(State.NextToken->is(tok::r_brace) && > State.Stack.back().BreakBeforeClosingBrace)) > return false; > + // This prevents breaks like: > + // ... > + // SomeParameter, OtherParameter).DoSomething( > + // ... > + // As they hide "DoSomething" and generally bad for readability. > + if (State.NextToken->Parent->is(tok::l_paren) && > + State.ParenLevel <= State.StartOfLineLevel) > + return false; > // Trying to insert a parameter on a new line if there are already more > than > // one parameter on the current line is bin packing. > if (State.Stack.back().HasMultiParameterLine && > > Modified: cfe/trunk/unittests/Format/FormatTest.cpp > URL: > http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=175432&r1=175431&r2=175432&view=diff > ============================================================================== > --- cfe/trunk/unittests/Format/FormatTest.cpp (original) > +++ cfe/trunk/unittests/Format/FormatTest.cpp Mon Feb 18 05:05:07 2013 > @@ -1457,6 +1457,13 @@ TEST_F(FormatTest, WrapsAtFunctionCallsI > verifyFormat("SomeMap[std::pair(aaaaaaaaaaaa, bbbbbbbbbbbbbbb)]\n" > " .insert(ccccccccccccccccccccccc);"); > > + verifyGoogleFormat( > + "aaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n" > + " .aaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaaaa)\n" > + " .aaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaaaaaaaaa,\n" > + " aaaaaaaaaaaaaaaaaaa,\n" > + " aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa);"); > + > // Here, it is not necessary to wrap at "." or "->". > verifyFormat("if (aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa(aaaaaaaaaaaa) > ||\n" > " aaaa.aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa) {\n}"); > > > _______________________________________________ > cfe-commits mailing list > [email protected] > http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
Format.cpp.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
