[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2022-01-03 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG1188f241acb7: Revert "[clang-format][NFC] Prefer pass by reference" (authored by HazardyKnusperkeks). Changed prior to commit: https://reviews.llvm.org/D115061?vs=391858&id=397137#toc Repository: rG

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-05 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks reopened this revision. HazardyKnusperkeks added a comment. This revision is now accepted and ready to land. I will revert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115061/new/ https://reviews.llvm.org/D115061 _

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. 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`... Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115061/new/ https://reviews.llvm.org/D

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. 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. Repository: rG LLVM Github Monorepo CHANGES SINCE

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-04 Thread Björn Schäpers via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG25f637913fe3: [clang-format][NFC] Prefer pass by reference (authored by HazardyKnusperkeks). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D115061/new/ https

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Owen Pan via Phabricator via cfe-commits
owenpan accepted this revision. owenpan added a comment. This revision is now accepted and ready to land. In D115061#3170908 , @HazardyKnusperkeks wrote: > No I don't think there is a performance penalty. **I** just don't like using > pointers. I'm happ

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks added a comment. 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.

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Arthur O'Dwyer via Phabricator via cfe-commits
Quuxplusone added a comment. 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, N

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Marek Kurdej via Phabricator via cfe-commits
curdeius added a comment. 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

[PATCH] D115061: [clang-format][NFC] Prefer pass by reference

2021-12-03 Thread Björn Schäpers via Phabricator via cfe-commits
HazardyKnusperkeks created this revision. HazardyKnusperkeks added reviewers: owenpan, MyDeveloperDay, curdeius. HazardyKnusperkeks added a project: clang-format. HazardyKnusperkeks requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Repository