llvmorg-github-actions[bot] wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang-format Author: Yash Aggarwal (yashaggarwal85d) <details> <summary>Changes</summary> The penalty in OptimizingLineFormatter::analyzeSolutionSpace() was summed in an unsigned. On a long braced list where every row goes over the column limit, the total goes past 2^32 and wraps, so a worse layout can end up with the smaller penalty. With 81 rows of 100-digit literals the brace moved to its own line and everything got indented 4 more. This keeps the search penalty (and OrderedPenalty) in 64 bits, and clamps the value returned from formatLine() to UINT_MAX so callers that add it up don't wrap either. Penalties of a single token can still overflow inside ContinuationIndenter if one token is thousands of columns past the limit. I left that alone to keep this small. Fixes #<!-- -->235788 --- Full diff: https://github.com/llvm/llvm-project/pull/236087.diff 2 Files Affected: - (modified) clang/lib/Format/UnwrappedLineFormatter.cpp (+7-5) - (modified) clang/unittests/Format/FormatTest.cpp (+9) ``````````diff diff --git a/clang/lib/Format/UnwrappedLineFormatter.cpp b/clang/lib/Format/UnwrappedLineFormatter.cpp index d1f68877ef0339..bb44d134f3edf4 100644 --- a/clang/lib/Format/UnwrappedLineFormatter.cpp +++ b/clang/lib/Format/UnwrappedLineFormatter.cpp @@ -1425,7 +1425,7 @@ class OptimizingLineFormatter : public LineFormatter { /// In case of equal penalties, we want to prefer states that were inserted /// first. During state generation we make sure that we insert states first /// that break the line as late as possible. - typedef std::pair<unsigned, unsigned> OrderedPenalty; + typedef std::pair<uint64_t, unsigned> OrderedPenalty; /// An edge in the solution space from \c Previous->State to \c State, /// inserting a newline dependent on the \c NewLine. @@ -1468,7 +1468,7 @@ class OptimizingLineFormatter : public LineFormatter { Queue.push(QueueItem(OrderedPenalty(0, Count), RootNode)); ++Count; - unsigned Penalty = 0; + uint64_t Penalty = 0; // While not empty, take first element and follow edges. while (!Queue.empty()) { @@ -1517,14 +1517,14 @@ class OptimizingLineFormatter : public LineFormatter { << "Total number of analyzed states: " << Count << "\n"); LLVM_DEBUG(llvm::dbgs() << "---\n"); - return Penalty; + return std::min<uint64_t>(Penalty, UINT_MAX); } /// Add the following state to the analysis queue \c Queue. /// /// Assume the current state is \p PreviousNode and has been reached with a /// penalty of \p Penalty. Insert a line break if \p NewLine is \c true. - void addNextStateToQueue(unsigned Penalty, StateNode *PreviousNode, + void addNextStateToQueue(uint64_t Penalty, StateNode *PreviousNode, bool NewLine, unsigned *Count, QueueType *Queue) { if (NewLine && !Indenter->canBreak(PreviousNode->State)) return; @@ -1533,9 +1533,11 @@ class OptimizingLineFormatter : public LineFormatter { StateNode *Node = new (Allocator.Allocate()) StateNode(PreviousNode->State, NewLine, PreviousNode); - if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty)) + unsigned ChildrenPenalty = 0; + if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, ChildrenPenalty)) return; + Penalty += ChildrenPenalty; Penalty += Indenter->addTokenToState(Node->State, NewLine, true); Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node)); diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp index f38287757ffba1..df971a4c513845 100644 --- a/clang/unittests/Format/FormatTest.cpp +++ b/clang/unittests/Format/FormatTest.cpp @@ -20440,6 +20440,15 @@ TEST_F(FormatTest, BreakPenaltyScopeResolution) { Style); } +TEST_F(FormatTest, PenaltyDoesNotOverflow) { + // The total penalty of this line is above 2^32. + std::string Code = "int t[] = {\n"; + for (int I = 0; I < 81; ++I) + Code += " 0x" + std::string(100, '0') + ",\n"; + Code += "};"; + verifyFormat(Code); +} + TEST_F(FormatTest, WorksFor8bitEncodings) { // FIXME: unstable test case EXPECT_EQ("\"\xce\xe4\xed\xe0\xe6\xe4\xfb \xe2 \"\n" `````````` </details> https://github.com/llvm/llvm-project/pull/236087 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
