Index: clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp =================================================================== --- clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp +++ clang-tools-extra/trunk/clang-tidy/hicpp/SignedBitwiseCheck.cpp @@ -20,34 +20,72 @@ void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) { const auto SignedIntegerOperand = - expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed_operand"); + expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed-operand"); + + // The standard [bitmask.types] allows some integral types to be implemented + // as signed types. Exclude these types from diagnosing for bitwise or(|) and + // bitwise and(&). Shifting and complementing such values is still not + // allowed. + const auto BitmaskType = namedDecl(anyOf( + hasName("::std::locale::category"), hasName("::std::ctype_base::mask"), + hasName("::std::ios_base::fmtflags"), hasName("::std::ios_base::iostate"), + hasName("::std::ios_base::openmode"))); + const auto IsStdBitmask = ignoringImpCasts(declRefExpr(hasType(BitmaskType))); // Match binary bitwise operations on signed integer arguments. Finder->addMatcher( - binaryOperator(allOf(anyOf(hasOperatorName("|"), hasOperatorName("&"), - hasOperatorName("^"), hasOperatorName("<<"), - hasOperatorName(">>")), + binaryOperator( + allOf(anyOf(hasOperatorName("^"), hasOperatorName("|"), + hasOperatorName("&")), + + unless(allOf(hasLHS(IsStdBitmask), hasRHS(IsStdBitmask))), + + hasEitherOperand(SignedIntegerOperand), + hasLHS(hasType(isInteger())), hasRHS(hasType(isInteger())))) + .bind("binary-no-sign-interference"), + this); + + // Shifting and complement is not allowed for any signed integer type because + // the sign bit may corrupt the result. + Finder->addMatcher( + binaryOperator(allOf(anyOf(hasOperatorName("<<"), hasOperatorName(">>")), hasEitherOperand(SignedIntegerOperand), hasLHS(hasType(isInteger())), hasRHS(hasType(isInteger())))) - .bind("binary_signed"), + .bind("binary-sign-interference"), this); // Match unary operations on signed integer types. Finder->addMatcher(unaryOperator(allOf(hasOperatorName("~"), hasUnaryOperand(SignedIntegerOperand))) - .bind("unary_signed"), + .bind("unary-signed"), this); } void SignedBitwiseCheck::check(const MatchFinder::MatchResult &Result) { const ast_matchers::BoundNodes &N = Result.Nodes; - const auto *SignedBinary = N.getNodeAs("binary_signed"); - const auto *SignedUnary = N.getNodeAs("unary_signed"); - const auto *SignedOperand = N.getNodeAs("signed_operand"); + const auto *SignedOperand = N.getNodeAs("signed-operand"); + assert(SignedOperand && + "No signed operand found in problematic bitwise operations"); + + bool IsUnary = false; + SourceLocation Location; + + if (const auto *UnaryOp = N.getNodeAs("unary-signed")) { + IsUnary = true; + Location = UnaryOp->getLocStart(); + } else { + if (const auto *BinaryOp = + N.getNodeAs("binary-no-sign-interference")) + Location = BinaryOp->getLocStart(); + else if (const auto *BinaryOp = + N.getNodeAs("binary-sign-interference")) + Location = BinaryOp->getLocStart(); + else + llvm_unreachable("unexpected matcher result"); + } - const bool IsUnary = SignedUnary != nullptr; - diag(IsUnary ? SignedUnary->getLocStart() : SignedBinary->getLocStart(), + diag(Location, "use of a signed integer operand with a %select{binary|unary}0 bitwise " "operator") << IsUnary << SignedOperand->getSourceRange(); Index: clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise-standard-types.h =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise-standard-types.h +++ clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise-standard-types.h @@ -0,0 +1,81 @@ +#pragma clang system_header + +// Implement standard types that are known to be defined as unsigned in some +// implementations like MSVC. +namespace std { +namespace locale { +enum category : int { + none = 0u, + collate = 1u << 1u, + ctype = 1u << 2u, + monetary = 1u << 3u, + numeric = 1u << 4u, + time = 1u << 5u, + messages = 1u << 6u, + all = none | collate | ctype | monetary | numeric | time | messages + // CHECK MESSAGES: [[@LINE-1]]:9: warning: use of a signed integer operand with a binary bitwise operator +}; +} // namespace locale + +namespace ctype_base { +enum mask : int { + space, + print, + cntrl, + upper, + lower, + alpha, + digit, + punct, + xdigit, + /* blank, // C++11 */ + alnum = alpha | digit, + // CHECK MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator + graph = alnum | punct + // CHECK MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator +}; +} // namespace ctype_base + +namespace ios_base { +enum fmtflags : int { + dec = 0u, + oct = 1u << 2u, + hex = 1u << 3u, + basefield = dec | oct | hex | 0u, + // CHECK MESSAGES: [[@LINE-1]]:15: warning: use of a signed integer operand with a binary bitwise operator + left = 1u << 4u, + right = 1u << 5u, + internal = 1u << 6u, + adjustfield = left | right | internal, + // CHECK MESSAGES: [[@LINE-1]]:17: warning: use of a signed integer operand with a binary bitwise operator + scientific = 1u << 7u, + fixed = 1u << 8u, + floatfield = scientific | fixed | (scientific | fixed) | 0u, + // CHECK MESSAGES: [[@LINE-1]]:16: warning: use of a signed integer operand with a binary bitwise operator + // CHECK MESSAGES: [[@LINE-2]]:38: warning: use of a signed integer operand with a binary bitwise operator + boolalpha = 1u << 9u, + showbase = 1u << 10u, + showpoint = 1u << 11u, + showpos = 1u << 12u, + skipws = 1u << 13u, + unitbuf = 1u << 14u, + uppercase = 1u << 15u +}; + +enum iostate : int { + goodbit = 0u, + badbit = 1u << 1u, + failbit = 1u << 2u, + eofbit = 1u << 3u +}; + +enum openmode : int { + app = 0u, + binary = 0u << 1u, + in = 0u << 2u, + out = 0u << 3u, + trunc = 0u << 4u, + ate = 0u << 5u +}; +} // namespace ios_base +} // namespace std Index: clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp =================================================================== --- clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp +++ clang-tools-extra/trunk/test/clang-tidy/hicpp-signed-bitwise-standard-types.cpp @@ -0,0 +1,188 @@ +// RUN: clang-tidy %s -checks='-*,hicpp-signed-bitwise' -- -std=c++11 + +#include "hicpp-signed-bitwise-standard-types.h" + +void pure_bitmask_types() { + // std::locale::category + int SResult = 0; + std::locale::category C = std::locale::category::ctype; + + SResult = std::locale::category::none | std::locale::category::collate; + SResult = std::locale::category::ctype & std::locale::category::monetary; + SResult = std::locale::category::numeric ^ std::locale::category::time; + SResult = std::locale::category::messages | std::locale::category::all; + + SResult = std::locale::category::all & C; + SResult = std::locale::category::all | C; + SResult = std::locale::category::all ^ C; + + // std::ctype_base::mask + std::ctype_base::mask M = std::ctype_base::mask::punct; + + SResult = std::ctype_base::mask::space | std::ctype_base::mask::print; + SResult = std::ctype_base::mask::cntrl & std::ctype_base::mask::upper; + SResult = std::ctype_base::mask::lower ^ std::ctype_base::mask::alpha; + SResult = std::ctype_base::mask::digit | std::ctype_base::mask::punct; + SResult = std::ctype_base::mask::xdigit & std::ctype_base::mask::alnum; + SResult = std::ctype_base::mask::alnum ^ std::ctype_base::mask::graph; + + SResult = std::ctype_base::mask::space & M; + SResult = std::ctype_base::mask::space | M; + SResult = std::ctype_base::mask::space ^ M; + + // std::ios_base::fmtflags + std::ios_base::fmtflags F = std::ios_base::fmtflags::floatfield; + + SResult = std::ios_base::fmtflags::dec | std::ios_base::fmtflags::oct; + SResult = std::ios_base::fmtflags::hex & std::ios_base::fmtflags::basefield; + SResult = std::ios_base::fmtflags::left ^ std::ios_base::fmtflags::right; + SResult = std::ios_base::fmtflags::internal | std::ios_base::fmtflags::adjustfield; + SResult = std::ios_base::fmtflags::scientific & std::ios_base::fmtflags::fixed; + SResult = std::ios_base::fmtflags::floatfield ^ std::ios_base::fmtflags::boolalpha; + SResult = std::ios_base::fmtflags::showbase | std::ios_base::fmtflags::showpoint; + SResult = std::ios_base::fmtflags::showpos & std::ios_base::fmtflags::skipws; + SResult = std::ios_base::fmtflags::unitbuf ^ std::ios_base::fmtflags::uppercase; + + SResult = std::ios_base::fmtflags::unitbuf | F; + SResult = std::ios_base::fmtflags::unitbuf & F; + SResult = std::ios_base::fmtflags::unitbuf ^ F; + + // std::ios_base::iostate + std::ios_base::iostate S = std::ios_base::iostate::goodbit; + + SResult = std::ios_base::iostate::goodbit | std::ios_base::iostate::badbit; + SResult = std::ios_base::iostate::failbit & std::ios_base::iostate::eofbit; + SResult = std::ios_base::iostate::failbit ^ std::ios_base::iostate::eofbit; + + SResult = std::ios_base::iostate::goodbit | S; + SResult = std::ios_base::iostate::goodbit & S; + SResult = std::ios_base::iostate::goodbit ^ S; + + // std::ios_base::openmode + std::ios_base::openmode B = std::ios_base::openmode::binary; + + SResult = std::ios_base::openmode::app | std::ios_base::openmode::binary; + SResult = std::ios_base::openmode::in & std::ios_base::openmode::out; + SResult = std::ios_base::openmode::trunc ^ std::ios_base::openmode::ate; + + SResult = std::ios_base::openmode::trunc | B; + SResult = std::ios_base::openmode::trunc & B; + SResult = std::ios_base::openmode::trunc ^ B; +} + +void still_forbidden() { + // std::locale::category + unsigned int UResult = 0u; + int SResult = 0; + + SResult = std::ctype_base::mask::print ^ 8u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + SResult = std::ctype_base::mask::cntrl | 8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + SResult = std::ctype_base::mask::upper & 8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + SResult = std::ctype_base::mask::lower ^ -8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = std::locale::category::collate << 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::locale::category::ctype << 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::locale::category::monetary >> 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::locale::category::numeric >> 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = ~std::locale::category::messages; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + SResult = ~std::locale::category::all; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + + // std::ctype_base::mask + UResult = std::ctype_base::mask::space | 8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ctype_base::mask::print & 8u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ctype_base::mask::cntrl ^ -8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = std::ctype_base::mask::upper << 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ctype_base::mask::lower << 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ctype_base::mask::alpha >> 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ctype_base::mask::digit >> 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = ~std::ctype_base::mask::punct; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + SResult = ~std::ctype_base::mask::xdigit; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + + // std::ios_base::fmtflags + UResult = std::ios_base::fmtflags::dec | 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::fmtflags::oct & 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::fmtflags::hex ^ -1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = std::ios_base::fmtflags::basefield >> 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::fmtflags::left >> 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::fmtflags::right << 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::fmtflags::internal << 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = ~std::ios_base::fmtflags::adjustfield; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + SResult = ~std::ios_base::fmtflags::scientific; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + + // std::ios_base::iostate + UResult = std::ios_base::iostate::goodbit | 8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::iostate::badbit & 8u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::iostate::failbit ^ -8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = std::ios_base::iostate::eofbit << 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::iostate::goodbit << 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::iostate::badbit >> 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::iostate::failbit >> 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = ~std::ios_base::iostate::eofbit; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + SResult = ~std::ios_base::iostate::goodbit; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + + // std::ios_base::openmode + UResult = std::ios_base::app | 8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::binary & 8u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::in ^ -8; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = std::ios_base::out >> 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::trunc >> 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::ate << 1; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + UResult = std::ios_base::ate << 1u; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator + + UResult = ~std::ios_base::openmode::app; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator + SResult = ~std::ios_base::openmode::binary; + // CHECK-MESSAGES: [[@LINE-1]]:13: warning: use of a signed integer operand with a unary bitwise operator +}