[clang-tools-extra] r356871 - [clang-tidy] Fix more false positives for bugprone-string-integer-assignment
Clement Courbet via cfe-commits
cfe-commits at lists.llvm.org
Mon Mar 25 01:18:00 PDT 2019
Author: courbet
Date: Mon Mar 25 01:18:00 2019
New Revision: 356871
URL: http://llvm.org/viewvc/llvm-project?rev=356871&view=rev
Log:
[clang-tidy] Fix more false positives for bugprone-string-integer-assignment
Summary:
And add various tests gleaned for our codebase.
See PR27723.
Reviewers: JonasToth, alexfh, xazax.hun
Subscribers: rnkovacs, jdoerfert, cfe-commits
Tags: #clang
Differential Revision: https://reviews.llvm.org/D59360
Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
Modified: clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp?rev=356871&r1=356870&r2=356871&view=diff
==============================================================================
--- clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp (original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/StringIntegerAssignmentCheck.cpp Mon Mar 25 01:18:00 2019
@@ -45,38 +45,100 @@ void StringIntegerAssignmentCheck::regis
this);
}
-static bool isLikelyCharExpression(const Expr *Argument,
- const ASTContext &Ctx) {
- const auto *BinOp = dyn_cast<BinaryOperator>(Argument);
- if (!BinOp)
+class CharExpressionDetector {
+public:
+ CharExpressionDetector(QualType CharType, const ASTContext &Ctx)
+ : CharType(CharType), Ctx(Ctx) {}
+
+ bool isLikelyCharExpression(const Expr *E) const {
+ if (isCharTyped(E))
+ return true;
+
+ if (const auto *BinOp = dyn_cast<BinaryOperator>(E)) {
+ const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
+ const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
+ // Handle both directions, e.g. `'a' + (i % 26)` and `(i % 26) + 'a'`.
+ if (BinOp->isAdditiveOp() || BinOp->isBitwiseOp())
+ return handleBinaryOp(BinOp->getOpcode(), LHS, RHS) ||
+ handleBinaryOp(BinOp->getOpcode(), RHS, LHS);
+ // Except in the case of '%'.
+ if (BinOp->getOpcode() == BO_Rem)
+ return handleBinaryOp(BinOp->getOpcode(), LHS, RHS);
+ return false;
+ }
+
+ // Ternary where at least one branch is a likely char expression, e.g.
+ // i < 265 ? i : ' '
+ if (const auto *CondOp = dyn_cast<AbstractConditionalOperator>(E))
+ return isLikelyCharExpression(
+ CondOp->getFalseExpr()->IgnoreParenImpCasts()) ||
+ isLikelyCharExpression(
+ CondOp->getTrueExpr()->IgnoreParenImpCasts());
return false;
- const auto *LHS = BinOp->getLHS()->IgnoreParenImpCasts();
- const auto *RHS = BinOp->getRHS()->IgnoreParenImpCasts();
- // <expr> & <mask>, mask is a compile time constant.
- Expr::EvalResult RHSVal;
- if (BinOp->getOpcode() == BO_And &&
- (RHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects) ||
- LHS->EvaluateAsInt(RHSVal, Ctx, Expr::SE_AllowSideEffects)))
- return true;
- // <char literal> + (<expr> % <mod>), where <base> is a char literal.
- const auto IsCharPlusModExpr = [](const Expr *L, const Expr *R) {
- const auto *ROp = dyn_cast<BinaryOperator>(R);
- return ROp && ROp->getOpcode() == BO_Rem && isa<CharacterLiteral>(L);
- };
- if (BinOp->getOpcode() == BO_Add) {
- if (IsCharPlusModExpr(LHS, RHS) || IsCharPlusModExpr(RHS, LHS))
+ }
+
+private:
+ bool handleBinaryOp(clang::BinaryOperatorKind Opcode, const Expr *const LHS,
+ const Expr *const RHS) const {
+ // <char_expr> <op> <char_expr> (c++ integer promotion rules make this an
+ // int), e.g.
+ // 'a' + c
+ if (isCharTyped(LHS) && isCharTyped(RHS))
return true;
+
+ // <expr> & <char_valued_constant> or <expr> % <char_valued_constant>, e.g.
+ // i & 0xff
+ if ((Opcode == BO_And || Opcode == BO_Rem) && isCharValuedConstant(RHS))
+ return true;
+
+ // <char_expr> | <char_valued_constant>, e.g.
+ // c | 0x80
+ if (Opcode == BO_Or && isCharTyped(LHS) && isCharValuedConstant(RHS))
+ return true;
+
+ // <char_constant> + <likely_char_expr>, e.g.
+ // 'a' + (i % 26)
+ if (Opcode == BO_Add)
+ return isCharConstant(LHS) && isLikelyCharExpression(RHS);
+
+ return false;
}
- return false;
-}
+
+ // Returns true if `E` is an character constant.
+ bool isCharConstant(const Expr *E) const {
+ return isCharTyped(E) && isCharValuedConstant(E);
+ };
+
+ // Returns true if `E` is an integer constant which fits in `CharType`.
+ bool isCharValuedConstant(const Expr *E) const {
+ if (E->isInstantiationDependent())
+ return false;
+ Expr::EvalResult EvalResult;
+ if (!E->EvaluateAsInt(EvalResult, Ctx, Expr::SE_AllowSideEffects))
+ return false;
+ return EvalResult.Val.getInt().getActiveBits() <= Ctx.getTypeSize(CharType);
+ };
+
+ // Returns true if `E` has the right character type.
+ bool isCharTyped(const Expr *E) const {
+ return E->getType().getCanonicalType().getTypePtr() ==
+ CharType.getTypePtr();
+ };
+
+ const QualType CharType;
+ const ASTContext &Ctx;
+};
void StringIntegerAssignmentCheck::check(
const MatchFinder::MatchResult &Result) {
const auto *Argument = Result.Nodes.getNodeAs<Expr>("expr");
+ const auto CharType =
+ Result.Nodes.getNodeAs<QualType>("type")->getCanonicalType();
SourceLocation Loc = Argument->getBeginLoc();
// Try to detect a few common expressions to reduce false positives.
- if (isLikelyCharExpression(Argument, *Result.Context))
+ if (CharExpressionDetector(CharType, *Result.Context)
+ .isLikelyCharExpression(Argument))
return;
auto Diag =
@@ -88,7 +150,6 @@ void StringIntegerAssignmentCheck::check
if (Loc.isMacroID())
return;
- auto CharType = *Result.Nodes.getNodeAs<QualType>("type");
bool IsWideCharType = CharType->isWideCharType();
if (!CharType->isCharType() && !IsWideCharType)
return;
Modified: clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp
URL: http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp?rev=356871&r1=356870&r2=356871&view=diff
==============================================================================
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-string-integer-assignment.cpp Mon Mar 25 01:18:00 2019
@@ -7,6 +7,8 @@ struct basic_string {
basic_string& operator=(basic_string);
basic_string& operator+=(T);
basic_string& operator+=(basic_string);
+ const T &operator[](int i) const;
+ T &operator[](int i);
};
typedef basic_string<char> string;
@@ -21,10 +23,13 @@ int toupper(int i);
typedef int MyArcaneChar;
+constexpr char kCharConstant = 'a';
+
int main() {
std::string s;
std::wstring ws;
int x = 5;
+ const char c = 'c';
s = 6;
// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: an integer is interpreted as a character code when assigning {{.*}} [bugprone-string-integer-assignment]
@@ -58,12 +63,47 @@ int main() {
s += toupper(x);
s += tolower(x);
- s += std::tolower(x);
+ s += (std::tolower(x));
+
+ s += c & s[1];
+ s += c ^ s[1];
+ s += c | s[1];
+
+ s[x] += 1;
+ s += s[x];
+ as += as[x];
// Likely character expressions.
s += x & 0xff;
s += 0xff & x;
+ s += x % 26;
+ s += 26 % x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+ // CHECK-FIXES: {{^}} s += std::to_string(26 % x);{{$}}
+ s += c | 0x80;
+ s += c | 0x8000;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+ // CHECK-FIXES: {{^}} s += std::to_string(c | 0x8000);{{$}}
+ as += c | 0x8000;
s += 'a' + (x % 26);
+ s += kCharConstant + (x % 26);
+ s += 'a' + (s[x] & 0xf);
s += (x % 10) + 'b';
+
+ s += x > 255 ? c : x;
+ s += x > 255 ? 12 : x;
+ // CHECK-MESSAGES: :[[@LINE-1]]:8: warning: an integer is interpreted as a chara
+ // CHECK-FIXES: {{^}} s += std::to_string(x > 255 ? 12 : x);{{$}}
+}
+
+namespace instantiation_dependent_exprs {
+template<typename T>
+struct S {
+ static constexpr T t = 0x8000;
+ std::string s;
+ void f(char c) { s += c | static_cast<int>(t); }
+};
+
+template S<int>;
}
More information about the cfe-commits
mailing list