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

Alexander Kornienko via cfe-commits cfe-commits at lists.llvm.org
Tue Apr 19 16:15:05 PDT 2016

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?


More information about the cfe-commits mailing list