xazax.hun added inline comments.
================ Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:30 + " The old user defined 'RandomFunction' is not usable for 'shuffle'. You " + "need to " + "make additional changes if you want a specific random function."; ---------------- madsravn wrote: > xazax.hun wrote: > > Maybe it would be worth to reflow this literal. > It seems a little weird, but this is the result of clang-format. I usually just make it one big line and than clang format will do a better reflow. By default it do not remove linebreaks, even if it could improve the layout. ================ Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:70 + llvm::raw_string_ostream Stream(Buffer); + DRef->printPretty(Stream, nullptr, Ctx.getPrintingPolicy()); + StringRef StrRef(Stream.str()); ---------------- madsravn wrote: > xazax.hun wrote: > > What about accessing the buffer of the source file instead of pretty > > printing? > How would you do this? printPretty was the best that I could find fitting my > needs. If you have something better fitting, please share :) I was thinking about something like getSourceText of Lexer. ================ Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:81 + Stream << "shuffle("; + FirstArgument->printPretty(Stream, nullptr, Ctx.getPrintingPolicy()); + Stream << ", "; ---------------- madsravn wrote: > xazax.hun wrote: > > Wouldn't just using the original source range of the argument work? > > What about preserving the comments next to the arguments? > I am not sure what you mean about the original source range. Can I just put > that onto the Stream? > > Do you have an idea for the comments? Do you mean something like > > > ``` > std::random_shuffle( > vec.begin(), // Comment > vec.end() // Comment > ); > ``` Or even: ``` std::random_shuffle(vec.begin(), /*inlinecomment*/ vec.end()); ``` ================ Comment at: clang-tidy/modernize/ReplaceRandomShuffleCheck.cpp:101 + + auto Diag = diag(MatchedCallExpr->getLocStart(), Message); + Diag << FixItHint::CreateRemoval(MatchedCallExpr->getSourceRange()); ---------------- madsravn wrote: > xazax.hun wrote: > > You do not want to do fixits for code that is the result of macro > > expansions. > Why not? And how would I fix that? Because that might cause the code not to compile when the macro is expanded in a different context. It is a conservative approach but other tidy checkers usually do this as well. You can use the isMacroID() method of the source locations. https://reviews.llvm.org/D30158 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits