[clang-tidy] hicpp bitwise operations on signed integers

Summary:
This check implements the rule [[ http://www.codingstandard.com/section/5-6-shift-operators/ | 5.6. HIC++ ]]
that forbidds bitwise operations on signed integer types.

Reviewers: aaron.ballman, hokein, alexfh, Eugene.Zelenko

Reviewed By: aaron.ballman

Subscribers: cfe-commits, mgorny, JDevlieghere, xazax.hun

Differential Revision: https://reviews.llvm.org/D36586

llvm-svn: 312122

clang-tools-extra/clang-tidy/hicpp/CMakeLists.txt

@@ -4,6 +4,7 @@ add_clang_library(clangTidyHICPPModule
   ExceptionBaseclassCheck.cpp
   NoAssemblerCheck.cpp
   HICPPTidyModule.cpp
+  SignedBitwiseCheck.cpp
 
   LINK_LIBS
   clangAST

clang-tools-extra/clang-tidy/hicpp/HICPPTidyModule.cpp

@@ -26,6 +26,7 @@
 #include "../readability/IdentifierNamingCheck.h"
 #include "ExceptionBaseclassCheck.h"
 #include "NoAssemblerCheck.h"
+#include "SignedBitwiseCheck.h"
 
 namespace clang {
 namespace tidy {
@@ -38,6 +39,8 @@ class HICPPModule : public ClangTidyModule {
         "hicpp-braces-around-statements");
     CheckFactories.registerCheck<ExceptionBaseclassCheck>(
         "hicpp-exception-baseclass");
+    CheckFactories.registerCheck<SignedBitwiseCheck>(
+        "hicpp-signed-bitwise");
     CheckFactories.registerCheck<google::ExplicitConstructorCheck>(
         "hicpp-explicit-conversions");
     CheckFactories.registerCheck<readability::FunctionSizeCheck>(

clang-tools-extra/clang-tidy/hicpp/SignedBitwiseCheck.cpp

@@ -0,0 +1,56 @@
+//===--- 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<BinaryOperator>("binary_signed");
+  const auto *SignedUnary = N.getNodeAs<UnaryOperator>("unary_signed");
+  const auto *SignedOperand = N.getNodeAs<Expr>("signed_operand");
+
+  const bool IsUnary = SignedUnary != nullptr;
+  diag(IsUnary ? SignedUnary->getLocStart() : SignedBinary->getLocStart(),
+       "use of a signed integer operand with a %select{binary|unary}0 bitwise "
+       "operator")
+      << IsUnary << SignedOperand->getSourceRange();
+}
+
+} // namespace hicpp
+} // namespace tidy
+} // namespace clang

clang-tools-extra/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

clang-tools-extra/docs/ReleaseNotes.rst

@@ -120,6 +120,12 @@ Improvements to clang-tidy
   Ensures that all exception will be instances of ``std::exception`` and classes 
   that are derived from it.
 
+- New `hicpp-signed-bitwise
+  <http://clang.llvm.org/extra/clang-tidy/checks/hicpp-signed-bitwise.html>`_ check
+
+  Finds uses of bitwise operations on signed integer types, which may lead to 
+  undefined or implementation defined behaviour.
+
 - New `android-cloexec-inotify-init1
   <http://clang.llvm.org/extra/clang-tidy/checks/android-cloexec-inotify-init1.html>`_ check
 

clang-tools-extra/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 <http://www.codingstandard.com/section/5-6-shift-operators/>`_.

clang-tools-extra/docs/clang-tidy/checks/list.rst

@@ -79,6 +79,7 @@ Clang-Tidy Checks
    hicpp-new-delete-operators (redirects to misc-new-delete-overloads) <hicpp-new-delete-operators>
    hicpp-no-assembler
    hicpp-noexcept-move (redirects to misc-noexcept-moveconstructor) <hicpp-noexcept-move>
+   hicpp-signed-bitwise
    hicpp-special-member-functions (redirects to cppcoreguidelines-special-member-functions) <hicpp-special-member-functions>
    hicpp-undelegated-constructor (redirects to misc-undelegated-constructor) <hicpp-undelegated-constructor>
    hicpp-use-equals-default (redirects to modernize-use-equals-default) <hicpp-use-equals-default>

clang-tools-extra/test/clang-tidy/hicpp-signed-bitwise.cpp

@@ -0,0 +1,219 @@
+// 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 binary_bitwise() {
+  int SValue = 42;
+  int SResult;
+
+  unsigned int UValue = 42;
+  unsigned int UResult;
+
+  SResult = SValue & 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  SResult = SValue & SValue;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  UResult = SValue & 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  UResult = SValue & -1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+
+  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;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  UByte1 = SByte1 & SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+  SByte1 = SByte1 & SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  // More complex expressions.
+  UResult = UValue & (SByte1 + (SByte1 | SByte2));
+  // CHECK-MESSAGES: :[[@LINE-1]]:13: warning: use of a signed integer operand with a binary bitwise operator
+  // CHECK-MESSAGES: :[[@LINE-2]]:33: warning: use of a signed integer operand with a binary bitwise operator
+
+  // 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;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 ^ UByte2; // Ok
+  UByte1 = UByte1 ^ SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 >> UByte2; // Ok
+  UByte1 = UByte1 >> SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  UByte1 = UByte1 << UByte2; // Ok
+  UByte1 = UByte1 << SByte2;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a binary bitwise operator
+
+  int SignedInt1 = 1 << 12;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  int SignedInt2 = 1u << 12;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+void f1(unsigned char c) {}
+void f2(char c) {}
+void f3(int c) {}
+
+void unary_bitwise() {
+  unsigned char UByte1 = 0u;
+  char SByte1 = 0;
+
+  UByte1 = ~UByte1; // Ok
+  SByte1 = ~UByte1;
+  SByte1 = ~SByte1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+  UByte1 = ~SByte1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:12: warning: use of a signed integer operand with a unary bitwise operator
+
+  unsigned int UInt = 0u;
+  int SInt = 0;
+
+  f1(~UByte1); // Ok
+  f1(~SByte1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use of a signed integer operand with a unary bitwise operator
+  f1(~UInt);
+  f1(~SInt);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use of a signed integer operand with a unary bitwise operator
+  f2(~UByte1);
+  f2(~SByte1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use of a signed integer operand with a unary bitwise operator
+  f2(~UInt);
+  f2(~SInt);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use of a signed integer operand with a unary bitwise operator
+  f3(~UByte1); // Ok
+  f3(~SByte1);
+  // CHECK-MESSAGES: :[[@LINE-1]]:6: warning: use of a signed integer operand with a unary bitwise operator
+}
+
+/// HICPP uses these examples to demonstrate the rule.
+void standard_examples() {
+  int i = 3;
+  unsigned int k = 0u;
+
+  int r = i << -1; // Emits -Wshift-count-negative from clang
+  // CHECK-MESSAGES: :[[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  r = i << 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+
+  r = -1 >> -1; // Emits -Wshift-count-negative from clang
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+  r = -1 >> 1;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+
+  r = -1 >> i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+  r = -1 >> -i;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+
+  r = ~0;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a unary bitwise operator
+  r = ~0u; // Ok
+  k = ~k;  // Ok
+
+  unsigned int u = (-1) & 2u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: use of a signed integer operand with a binary bitwise operator
+  u = (-1) | 1u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+  u = (-1) ^ 1u;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+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
+}
+
+enum OldEnum {
+  ValueOne,
+  ValueTwo,
+};
+
+enum OldSigned : int {
+  IntOne,
+  IntTwo,
+};
+
+void classicEnums() {
+  OldEnum e1 = ValueOne, e2 = ValueTwo;
+  int e3;                   // Using the enum type, results in an error.
+  e3 = ValueOne | ValueTwo; // Ok
+  e3 = ValueOne & ValueTwo; // Ok
+  e3 = ValueOne ^ ValueTwo; // Ok
+  e3 = e1 | e2;             // Ok
+  e3 = e1 & e2;             // Ok
+  e3 = e1 ^ e2;             // Ok
+
+  OldSigned s1 = IntOne, s2 = IntTwo;
+  int s3;
+  s3 = IntOne | IntTwo; // Signed
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator
+  s3 = IntOne & IntTwo; // Signed
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator
+  s3 = IntOne ^ IntTwo; // Signed
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator
+  s3 = s1 | s2; // Signed
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator
+  s3 = s1 & s2; // Signed
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator
+  s3 = s1 ^ s2; // Signed
+  // CHECK-MESSAGES: [[@LINE-1]]:8: warning: use of a signed integer operand with a binary bitwise operator
+}
+
+enum EnumConstruction {
+  one = 1,
+  two = 2,
+  test1 = 1 << 12,
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  test2 = one << two,
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+  test3 = 1u << 12,
+  // CHECK-MESSAGES: [[@LINE-1]]:11: warning: use of a signed integer operand with a binary bitwise operator
+};