aaron.ballman added inline comments.
================
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:77
+
+ auto Diag = [=]() {
+ std::string Message = ReplaceMessage;
----------------
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.
================
Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:89
+
+ std::string newname = "shuffle";
+ StringRef ContainerText = Lexer::getSourceText(
----------------
Should be `NewName` instead.
https://reviews.llvm.org/D30158
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
