PiotrZSL requested changes to this revision. PiotrZSL added a comment. This revision now requires changes to proceed.
I run into false positive with this example: using UInt8 = unsigned char; UInt8 size = 5U; std::string str2(size, 'x'); // warning: string constructor arguments are probably swapped; expecting string(count, character) [bugprone-string-constructor] This is due to change in CharExpr, simply isAnyCharacter is too wide, as it also match signed/unsigned characters, and your tests does not cover those. Probably for this we need to be more strict, and accept only non signed/unsigned characters. Other issue is with this: char c = '\n'; using Size = int; Size size = 10U; std::string str2(c, size); Even that in AST we got double implicit casts, it's detected as `warning: string constructor arguments are probably swapped; expecting string(count, character) [bugprone-string-constructor]`. Instead of being detected as 'confusing string fill constructor arguments'. This probably could be fixed by matching those double casts first, instead of last. | `-VarDecl 0x56436ceb0520 <col:3, col:27> col:15 str2 'std::string':'std::basic_string<char>' callinit | `-CXXConstructExpr 0x56436ceb0650 <col:15, col:27> 'std::string':'std::basic_string<char>' 'void (unsigned int, char)' | |-ImplicitCastExpr 0x56436ceb0608 <col:20> 'unsigned int' <IntegralCast> | | `-ImplicitCastExpr 0x56436ceb05f0 <col:20> 'char' <LValueToRValue> | | `-DeclRefExpr 0x56436ceb0588 <col:20> 'char' lvalue Var 0x56436ceb0280 'c' 'char' | `-ImplicitCastExpr 0x56436ceb0638 <col:23> 'char':'char' <IntegralCast> | `-ImplicitCastExpr 0x56436ceb0620 <col:23> 'Size':'int' <LValueToRValue> | `-DeclRefExpr 0x56436ceb05a8 <col:23> 'Size':'int' lvalue Var 0x56436ceb0420 'size' 'Size':'int' Except above 2 change looks OK to me. My main concern is false-positive, simply because in project that I work for we use lot of std::uint8_t as size, to save bytes in struct. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D143971/new/ https://reviews.llvm.org/D143971 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
