aaron.ballman added inline comments.
================
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+ auto Diag = [=]() {
+ std::string Message = ReplaceMessage;
----------------
madsravn wrote:
> 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.
It's entirely possible I'm missing something (I'm distracted with meetings this
week), but I was envisioning:
```
DiagnosticBuilder Diag;
if (MatchedCallExpr->getNumArgs() == 3) {
Diag =
diag(MatchedCallExpr->getLocStart(),
"'std::random_shuffle' has been removed in C++17; use "
"'std::shuffle' and an alternative random mechanism instead");
Diag << FixItHint::CreateReplacement(
MatchedArgumentThree->getSourceRange(),
"std::mt19937(std::random_device()())");
} else {
Diag = diag(MatchedCallExpr->getLocStart(),
"'std::random_shuffle' has been removed in C++17; use "
"'std::shuffle' instead");
Diag << FixItHint::CreateInsertion(
MatchedCallExpr->getRParenLoc(),
", std::mt19937(std::random_device()())");
}
```
https://reviews.llvm.org/D30158
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
