[PATCH] An attempt to make the search algorithm easier to understand.
Daniel Jasper
djasper at google.com
Tue Feb 12 23:26:24 PST 2013
Very nice! I think a few names could be more intuitive, but other than that looks awesome!
================
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;
----------------
How about: "This is used to create a deterministic order independent of the container."
================
Comment at: lib/Format/Format.cpp:654
@@ +653,3 @@
+ /// that break the line as late as possible.
+ typedef std::pair<unsigned, unsigned> StablePenalty;
+
----------------
Maybe OrderedPenalty?
================
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;
----------------
Incomplete comment.
================
Comment at: lib/Format/Format.cpp:718
@@ +717,3 @@
+
+ void apply(LineState &State, StateEdge *Current) {
+ if (Current->Previous == NULL)
----------------
apply seems a very generic name. How about useEdge?
================
Comment at: lib/Format/Format.cpp:738
@@ +737,3 @@
+ QueueType &Queue, unsigned &Count, unsigned Penalty,
+ StateEdge *Old, bool NewLine) {
+ if (NewLine && !canBreak(Old->State))
----------------
"Old" does not mean much to me. Maybe PreviousEdge.
================
Comment at: lib/Format/Format.cpp:644
@@ +643,3 @@
+ }
+ LineState State;
+ bool NewLine;
----------------
Might be nice to reflect in the name that this is the node that this edge leads to. Maybe "NextState"?
http://llvm-reviews.chandlerc.com/D398
BRANCH
format-penalty
ARCANIST PROJECT
clang
More information about the cfe-commits
mailing list