[PATCH] D19146: [clang-tidy] New checker to detect suspicious string constructor.

[Alexander Kornienko via cfe-commits]

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