[PATCH] An attempt to make the search algorithm easier to understand.

[Manuel Klimek]

  Any opinions on making the search data structures class members (we only support a single search per UnwrappedLineFormatter anyway)?


================
Comment at: lib/Format/Format.cpp:673
@@ +672,3 @@
+    // Increasing count of \c StateEdge items we have created. This is used
+    // to stabilize the priority queue.
+    unsigned Count = 0;
----------------
Daniel Jasper wrote:
> How about: "This is used to create a deterministic order independent of the container."
Done.

================
Comment at: lib/Format/Format.cpp:644
@@ +643,3 @@
+    }
+    LineState State;
+    bool NewLine;
----------------
Daniel Jasper wrote:
> Might be nice to reflect in the name that this is the node that this edge leads to. Maybe "NextState"?
Interesting view. I actually see this as a node with an *incoming* edge :) (that's why the link is called Previous)

================
Comment at: lib/Format/Format.cpp:654
@@ +653,3 @@
+  /// that break the line as late as possible.
+  typedef std::pair<unsigned, unsigned> StablePenalty;
+
----------------
Daniel Jasper wrote:
> Maybe OrderedPenalty?
Done.

================
Comment at: lib/Format/Format.cpp:656
@@ +655,3 @@
+
+  /// \brief An item in the prioritized BFS search queue. The \c StateEdge 
+  typedef std::pair<StablePenalty, StateEdge *> QueueItem;
----------------
Daniel Jasper wrote:
> Incomplete comment.
Done.

================
Comment at: lib/Format/Format.cpp:718
@@ +717,3 @@
+
+  void apply(LineState &State, StateEdge *Current) {
+    if (Current->Previous == NULL)
----------------
Daniel Jasper wrote:
> apply seems a very generic name. How about useEdge?
useEdge seems misleading, as it's not only applying a single edge, it's replaying the whole sequence. Perhaps reconstructPath, or applyPath, or generateEditsForPath?

================
Comment at: lib/Format/Format.cpp:738
@@ +737,3 @@
+                           QueueType &Queue, unsigned &Count, unsigned Penalty,
+                           StateEdge *Old, bool NewLine) {
+    if (NewLine && !canBreak(Old->State))
----------------
Daniel Jasper wrote:
> "Old" does not mean much to me. Maybe PreviousEdge.
Done.


http://llvm-reviews.chandlerc.com/D398

BRANCH
  format-penalty

ARCANIST PROJECT
  clang