Index: clang-tidy/hicpp/CMakeLists.txt =================================================================== --- clang-tidy/hicpp/CMakeLists.txt +++ clang-tidy/hicpp/CMakeLists.txt @@ -3,6 +3,7 @@ add_clang_library(clangTidyHICPPModule NoAssemblerCheck.cpp HICPPTidyModule.cpp + SignedBitwiseCheck.cpp LINK_LIBS clangAST Index: clang-tidy/hicpp/HICPPTidyModule.cpp =================================================================== --- clang-tidy/hicpp/HICPPTidyModule.cpp +++ clang-tidy/hicpp/HICPPTidyModule.cpp @@ -24,6 +24,7 @@ #include "../readability/FunctionSizeCheck.h" #include "../readability/IdentifierNamingCheck.h" #include "NoAssemblerCheck.h" +#include "SignedBitwiseCheck.h" namespace clang { namespace tidy { @@ -32,6 +33,8 @@ class HICPPModule : public ClangTidyModule { public: void addCheckFactories(ClangTidyCheckFactories &CheckFactories) override { + CheckFactories.registerCheck( + "hicpp-signed-bitwise"); CheckFactories.registerCheck( "hicpp-explicit-conversions"); CheckFactories.registerCheck( Index: clang-tidy/hicpp/SignedBitwiseCheck.h =================================================================== --- /dev/null +++ clang-tidy/hicpp/SignedBitwiseCheck.h @@ -0,0 +1,36 @@ +//===--- SignedBitwiseCheck.h - clang-tidy-----------------------*- C++ -*-===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_SIGNED_BITWISE_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_SIGNED_BITWISE_H + +#include "../ClangTidy.h" + +namespace clang { +namespace tidy { +namespace hicpp { + +/// This check implements the rule 5.6.1 of the HICPP Standard, which disallows +/// bitwise operations on signed integer types. +/// +/// For the user-facing documentation see: +/// http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html +class SignedBitwiseCheck : public ClangTidyCheck { +public: + SignedBitwiseCheck(StringRef Name, ClangTidyContext *Context) + : ClangTidyCheck(Name, Context) {} + void registerMatchers(ast_matchers::MatchFinder *Finder) override; + void check(const ast_matchers::MatchFinder::MatchResult &Result) override; +}; + +} // namespace hicpp +} // namespace tidy +} // namespace clang + +#endif // LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_HICPP_SIGNED_BITWISE_H Index: clang-tidy/hicpp/SignedBitwiseCheck.cpp =================================================================== --- /dev/null +++ clang-tidy/hicpp/SignedBitwiseCheck.cpp @@ -0,0 +1,55 @@ +//===--- SignedBitwiseCheck.cpp - clang-tidy-------------------------------===// +// +// The LLVM Compiler Infrastructure +// +// This file is distributed under the University of Illinois Open Source +// License. See LICENSE.TXT for details. +// +//===----------------------------------------------------------------------===// + +#include "SignedBitwiseCheck.h" +#include "clang/AST/ASTContext.h" +#include "clang/ASTMatchers/ASTMatchFinder.h" + +using namespace clang::ast_matchers; +using namespace clang::ast_matchers::internal; + +namespace clang { +namespace tidy { +namespace hicpp { + +void SignedBitwiseCheck::registerMatchers(MatchFinder *Finder) { + const auto SignedIntegerOperand = + expr(ignoringImpCasts(hasType(isSignedInteger()))).bind("signed_operand"); + + // Match binary bitwise operations on signed integer arguments. + Finder->addMatcher( + binaryOperator(allOf(anyOf(hasOperatorName("|"), hasOperatorName("&"), + hasOperatorName("^"), hasOperatorName("<<"), + hasOperatorName(">>")), + hasEitherOperand(SignedIntegerOperand))) + .bind("binary_signed"), + this); + + // Match unary operations on signed integer types. + Finder->addMatcher(unaryOperator(allOf(hasOperatorName("~"), + hasUnaryOperand(SignedIntegerOperand))) + .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 bool IsBinary = (SignedBinary == nullptr); + diag(IsBinary ? SignedUnary->getLocStart() : SignedBinary->getLocStart(), + "%select{unary|binary}0 bitwise operation with signed integers detected") + << IsBinary << SignedOperand->getSourceRange(); +} + +} // namespace hicpp +} // namespace tidy +} // namespace clang Index: docs/ReleaseNotes.rst =================================================================== --- docs/ReleaseNotes.rst +++ docs/ReleaseNotes.rst @@ -57,7 +57,7 @@ Improvements to clang-tidy -------------------------- -- Renamed checks to use correct term "implicit conversion" instead of "implicit +* Renamed checks to use correct term "implicit conversion" instead of "implicit cast" and modified messages and option names accordingly: * **performance-implicit-cast-in-loop** was renamed to @@ -76,6 +76,12 @@ Finds cases where integer division in a floating point context is likely to cause unintended loss of precision. +- New `hicpp-signed-bitwise + `_ check + + Finds uses of bitwise operations on signed integer types, which may lead to + undefined or implementation defined behaviour. + - New `readability-static-accessed-through-instance `_ check Index: docs/clang-tidy/checks/hicpp-signed-bitwise.rst =================================================================== --- /dev/null +++ docs/clang-tidy/checks/hicpp-signed-bitwise.rst @@ -0,0 +1,9 @@ +.. title:: clang-tidy - hicpp-signed-bitwise + +hicpp-signed-bitwise +==================== + +Finds uses of bitwise operations on signed integer types, which may lead to +undefined or implementation defined behaviour. + +The according rule is defined in the `High Integrity C++ Standard, Section 5.6.1 `_. Index: docs/clang-tidy/checks/list.rst =================================================================== --- docs/clang-tidy/checks/list.rst +++ docs/clang-tidy/checks/list.rst @@ -30,7 +30,7 @@ cert-msc30-c (redirects to cert-msc50-cpp) cert-msc50-cpp cert-oop11-cpp (redirects to misc-move-constructor-init) - cppcoreguidelines-c-copy-assignment-signature + cppcoreguidelines-c-copy-assignment-signature (redirects to misc-unconventional-assign-operator) cppcoreguidelines-interfaces-global-init cppcoreguidelines-no-malloc cppcoreguidelines-pro-bounds-array-to-pointer-decay @@ -69,6 +69,7 @@ hicpp-new-delete-operators (redirects to misc-new-delete-overloads) hicpp-no-assembler hicpp-noexcept-move (redirects to misc-noexcept-moveconstructor) + hicpp-signed-bitwise hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) hicpp-undelegated-constructor (redirects to misc-undelegated-constructor) hicpp-use-equals-default (redirects to modernize-use-equals-default) Index: test/clang-tidy/hicpp-signed-bitwise.cpp =================================================================== --- /dev/null +++ test/clang-tidy/hicpp-signed-bitwise.cpp @@ -0,0 +1,164 @@ +// RUN: %check_clang_tidy %s hicpp-signed-bitwise %t + +// These could cause false positives and should not be considered. +struct StreamClass { +}; +StreamClass &operator<<(StreamClass &os, unsigned int i) { + return os; +} +StreamClass &operator<<(StreamClass &os, int i) { + return os; +} +StreamClass &operator>>(StreamClass &os, unsigned int i) { + return os; +} +StreamClass &operator>>(StreamClass &os, int i) { + return os; +} +struct AnotherStream { + AnotherStream &operator<<(unsigned char c) { return *this; } + AnotherStream &operator<<(char c) { return *this; } + + AnotherStream &operator>>(unsigned char c) { return *this; } + AnotherStream &operator>>(char c) { return *this; } +}; + +void f1(unsigned char c) {} +void f2(char c) {} +void f3(int c) {} + +void binary_bitwise() { + int SValue = 42; + int SResult; + + unsigned int UValue = 42; + unsigned int UResult; + + SResult = SValue & 1; // Bad, one operand signed and result is signed, maybe fix with suffix + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binary bitwise operation with signed integers detected + SResult = SValue & -1; // Bad, both are signed and result is signed + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binary bitwise operation with signed integers detected + SResult = SValue & SValue; // Bad, both are sigend and result is signed + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binary bitwise operation with signed integers detected + + UResult = SValue & 1; // Bad, operation on signed, maybe fix with suffix + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binary bitwise operation with signed integers detected + UResult = SValue & -1; // Bad, operations are signed, and even negative + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binary bitwise operation with signed integers detected + + UResult = UValue & 1u; // Ok + UResult = UValue & UValue; // Ok + + unsigned char UByte1 = 0u; + unsigned char UByte2 = 16u; + char SByte1 = 0; + char SByte2 = 16; + + UByte1 = UByte1 & UByte2; // Ok + UByte1 = SByte1 & UByte2; // Bad, one is signed + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected + UByte1 = SByte1 & SByte2; // Bad, both are signed + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected + SByte1 = SByte1 & SByte2; // Bad, both are signed and result is signed + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected + + // More complex expressions. + UResult = UValue & (SByte1 + (SByte1 | SByte2)); // Bad, RHS is signed + // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: binary bitwise operation with signed integers detected + // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: binary bitwise operation with signed integers detected + + // The rest is to demonstrate functionality but all operators are matched equally. + // Therefore functionality is the same for all binary operations. + UByte1 = UByte1 | UByte2; // Ok + UByte1 = UByte1 | SByte2; // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected + + UByte1 = UByte1 ^ UByte2; // Ok + UByte1 = UByte1 ^ SByte2; // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected + + UByte1 = UByte1 >> UByte2; // Ok + UByte1 = UByte1 >> SByte2; // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected + + UByte1 = UByte1 << UByte2; // Ok + UByte1 = UByte1 << SByte2; // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: binary bitwise operation with signed integers detected +} + +void unary_bitwise() { + unsigned char UByte1 = 0u; + char SByte1 = 0; + + UByte1 = ~UByte1; // Ok + SByte1 = ~UByte1; // Bad? + SByte1 = ~SByte1; // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unary bitwise operation with signed integers detected + UByte1 = ~SByte1; // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: unary bitwise operation with signed integers detected + + unsigned int UInt = 0u; + int SInt = 0; + + f1(~UByte1); // Ok + f1(~SByte1); // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: unary bitwise operation with signed integers detected + f1(~UInt); // Bad, data loss + f1(~SInt); // Bad, data loss and Signed + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: unary bitwise operation with signed integers detected + f2(~UByte1); // Bad + f2(~SByte1); // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: unary bitwise operation with signed integers detected + f3(~UByte1); // Ok + f3(~SByte1); // Bad + // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: unary bitwise operation with signed integers detected +} + +/// HICPP uses these examples to demonstrate the rule. +void standard_examples() { + int i = 3; + unsigned int k = 0u; + + int r = i << -1; + // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: binary bitwise operation with signed integers detected + r = i << 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected + + r = -1 >> -1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected + r = -1 >> 1; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected + + r = -1 >> i; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected + r = -1 >> -i; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected + + r = ~0; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: unary bitwise operation with signed integers detected + r = ~0u; // Ok + k = ~k; // Ok + + unsigned int u = (-1) & 2u; + // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: binary bitwise operation with signed integers detected + u = (-1) | 1u; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected + u = (-1) ^ 1u; + // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: binary bitwise operation with signed integers detected +} + +void streams_should_work() { + StreamClass s; + s << 1u; // Ok + s << 1; // Ok + s >> 1; // Ok + s >> 1u; // Ok + + AnotherStream as; + unsigned char uc = 1u; + char sc = 1; + as << uc; // Ok + as << sc; // Ok + as >> uc; // Ok + as >> sc; // Ok +}