etienneb added inline comments.
================
Comment at: clang-tidy/misc/StringConstructorCheck.cpp:110
@@ -106,2 +109,3 @@
if (Result.Nodes.getNodeAs<Expr>("swapped-parameter")) {
- diag(Loc, "constructor parameters are probably swapped");
+ assert(E->getNumArgs() == 2 && "Invalid number of arguments");
+ auto D = diag(Loc, "constructor parameters are probably swapped");
----------------
alexfh wrote:
> Assertions should verify logical bugs in the code, not some properties of the
> user input. The matcher doesn't seem to ensure that the argument count is
> always 2, so it's possible to create code that will trigger this assertion.
> Either add `argumentCountIs(2)` to the matcher or remove the assertion.
As I get it, it's not needed to add ' argumentCountIs(2)' because:
hasArgument(0, hasType(CharPtrType)),
hasArgument(1, hasType(isInteger())),
the code already validates that there is at least two parameters.
The assert would fail if there is more parameters, which can occur only on a
incorrect definition of the constructor.
So, I'm i favor to just remove the assert.
http://reviews.llvm.org/D19547
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
