[clang-format][NFC] Prefer pass by reference

[curdeius]

What do we usually do for output parameters?
I'm ok with both refs and pointers. It seems to me to be google-style thingy to pass by pointer.
It is indeed clearer at the caller site that the passed variable will be modified.
Are you worried about any performance penalty when using pointers? If so, adding nonnull attribute or sth like this may help.

[• Quuxplusone]

FWLIW, I'm strongly in favor of "Pass out-parameters by pointer," for the reason Marek said (and the reason Google, Bloomberg, Facebook, Mongo, etc, do it) — it makes life easier for the reader of the calling code. Especially for e.g. addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue), I don't think it's at all obvious that this is going to modify the value of Count!
But this isn't my code and I don't know what LLVM's house style is.

[HazardyKnusperkeks]

No I don't think there is a performance penalty. I just don't like using pointers. I'm happy to drop this, if the Style says to use pointers. But from what I've seen in clang-format pointers are mostly used when it can be null, but there is a wild mix.

[owenpan]

In D115061#3170908, @HazardyKnusperkeks wrote:

No I don't think there is a performance penalty. I just don't like using pointers. I'm happy to drop this, if the Style says to use pointers. But from what I've seen in clang-format pointers are mostly used when it can be null, but there is a wild mix.

I don't think LLVM Coding Standards specify it one way or the other, so it LGTM.

[HazardyKnusperkeks]

This was not intended to be pushed, I forgot to move it out of the chunk. If we come to the conclusion that we want pointers I will revert this. If no one objects in the next say 3 days I will leave it so.

[curdeius]

I share Arthur's point of view and I prefer the pointers. I don't see an advantage of this proposed change.
If only we had C#'s out...

[HazardyKnusperkeks]

I will revert.

clang/lib/Format/UnwrappedLineFormatter.cpp

while (!Queue.empty()) {
         Node->State.IgnoreStackForComparison = true;
 
       if (!Seen.insert(&Node->State).second)
         // State already examined with lower penalty.
         continue;
 
       FormatDecision LastFormat = Node->State.NextToken->getDecision();
       if (LastFormat == FD_Unformatted || LastFormat == FD_Continue)
-        addNextStateToQueue(Penalty, Node, /*NewLine=*/false, Count, Queue);
+        addNextStateToQueue(Penalty, Node, /*NewLine=*/false, &Count, &Queue);
       if (LastFormat == FD_Unformatted || LastFormat == FD_Break)
-        addNextStateToQueue(Penalty, Node, /*NewLine=*/true, Count, Queue);
+        addNextStateToQueue(Penalty, Node, /*NewLine=*/true, &Count, &Queue);
     }
 
     if (Queue.empty()) {
       // We were unable to find a solution, do nothing.
       // FIXME: Add diagnostic?
       LLVM_DEBUG(llvm::dbgs() << "Could not find a solution.\n");
       return 0;
     }
unsigned analyzeSolutionSpace(LineState &InitialState, bool DryRun) {
     return Penalty;
   }
 
   /// 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,
-                           bool NewLine, unsigned &Count, QueueType &Queue) {
+                           bool NewLine, unsigned *Count, QueueType *Queue) {
     if (NewLine && !Indenter->canBreak(PreviousNode->State))
       return;
     if (!NewLine && Indenter->mustBreak(PreviousNode->State))
       return;
 
     StateNode *Node = new (Allocator.Allocate())
         StateNode(PreviousNode->State, NewLine, PreviousNode);
     if (!formatChildren(Node->State, NewLine, /*DryRun=*/true, Penalty))
       return;
 
     Penalty += Indenter->addTokenToState(Node->State, NewLine, true);
 
-    Queue.push(QueueItem(OrderedPenalty(Penalty, Count), Node));
-    ++Count;
+    Queue->push(QueueItem(OrderedPenalty(Penalty, *Count), Node));
+    ++(*Count);
   }
 
   /// Applies the best formatting by reconstructing the path in the
   /// solution space that leads to \c Best.
   void reconstructPath(LineState &State, StateNode *Best) {
     std::deque<StateNode *> Path;
     // We do not need a break before the initial token.
     while (Best->Previous) {