[PATCH] D30158: [clang-tidy] modernize: Find usage of random_shuffle and replace it with shuffle.
Mads Ravn via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Wed Mar 1 11:16:19 PST 2017
madsravn marked an inline comment as done.
madsravn added inline comments.
================
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+ auto Diag = [=]() {
+ std::string Message = ReplaceMessage;
----------------
aaron.ballman wrote:
> madsravn wrote:
> > aaron.ballman wrote:
> > > Is there a reason this needs to capture everything by copy? Also, no need for the empty parens. Actually, is the lambda even necessary at all?
> > Is it OK to capture by reference then? Or how do we want it in llvm?
> >
> > We need the lambda, because first I need to create the diag with a message based on the count of arguments and then I need to find fixits based on the same count. Example:
> >
> >
> > ```
> > string message = "Message for 2 arguments";
> > if(argumentCount == 3) {
> > message = "Message for 3 arguments";
> > }
> > auto Diag = diag(startLoc(), message);
> > if(argumentCount == 3) {
> > Diag << FixitHint::FixForThreeArguments();
> > } else {
> > Diag << FixitHint::FixForTwoArguments();
> > }
> > ```
> >
> > So the idea with the lambda is to avoid doing the same if-statement twice.
> But you call the lambda immediately rather than store it and reuse it? It seems like you should be able to hoist a `DiagnosticBuilder` variable outside of the if statement and skip the lambda entirely.
I am not sure what you mean by this. Can you elaborate? Can you give a short example how I would hoist a `DiagnosticBuilder` out?
I think I tried something like that, but it was not an option.
https://reviews.llvm.org/D30158
More information about the cfe-commits
mailing list