alexfh accepted this revision. alexfh added a comment. This revision is now accepted and ready to land.
Looks good with a nit. In http://reviews.llvm.org/D19146#403234, @alexfh wrote: > In http://reviews.llvm.org/D19146#402414, @alexfh wrote: > > > In http://reviews.llvm.org/D19146#402410, @alexfh wrote: > > > > > I wonder whether `misc-swapped-arguments` can be tuned to handle these > > > cases in a more generic way? The only missing part so far seems to be the > > > lack of `cxxConstructExpr` support. > > > > > > And by "these cases" I mean the swapped arguments mistake, not the more > > string-specific ones. > > > What about this comment? As per offline discussion, there might be value in having the check for swapped std::string ctor arguments here, since this specialized check is likely to have much lower false positive rate than the more generic misc-swapped-arguments check. ================ Comment at: clang-tidy/misc/StringConstructorCheck.cpp:116 @@ +115,3 @@ + } else if (Result.Nodes.getNodeAs<Expr>("literal-with-length")) { + const auto *S = Result.Nodes.getNodeAs<StringLiteral>("str"); + const auto *L = Result.Nodes.getNodeAs<IntegerLiteral>("int"); ---------------- `S` and `L` are neither descriptive nor seem to be a commonly used name for this purpose. Can you expand variable names a bit? http://reviews.llvm.org/D19146 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
