Index: include/clang/Basic/DiagnosticGroups.td =================================================================== --- include/clang/Basic/DiagnosticGroups.td +++ include/clang/Basic/DiagnosticGroups.td @@ -233,6 +233,7 @@ def CXX11LongLong : DiagGroup<"c++11-long-long">; def LongLong : DiagGroup<"long-long", [CXX11LongLong]>; def MethodSignatures : DiagGroup<"method-signatures">; +def MismatchedOperators : DiagGroup<"mismatched-operators">; def MismatchedParameterTypes : DiagGroup<"mismatched-parameter-types">; def MismatchedReturnTypes : DiagGroup<"mismatched-return-types">; def MismatchedTags : DiagGroup<"mismatched-tags">; Index: include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- include/clang/Basic/DiagnosticSemaKinds.td +++ include/clang/Basic/DiagnosticSemaKinds.td @@ -36,6 +36,15 @@ "been assigned">, InGroup>, DefaultIgnore; def note_duplicate_element : Note<"element %0 also has value %1">; +def warn_wrong_binary_operator : Warning< + "using %select{bitwise|logical}0 operator when both operands are " + "%select{logical|bitwise}0">, InGroup, DefaultIgnore; +def warn_mismatch_operators : Warning< + "using %select{bitwise|logical}0 %select{and|or}1 operator with " + "%select{logical|bitwise}0 not">, InGroup, DefaultIgnore; +def note_suggest_operator : Note< + "did you mean to use %select{bitwise|logical}0 %select{and|or|not}1?">; + // Absolute value functions def warn_unsigned_abs : Warning< "taking the absolute value of unsigned type %0 has no effect">, Index: lib/Sema/SemaExpr.cpp =================================================================== --- lib/Sema/SemaExpr.cpp +++ lib/Sema/SemaExpr.cpp @@ -10097,6 +10097,154 @@ DiagnoseShiftCompare(Self, OpLoc, LHSExpr, RHSExpr); } +namespace { + +enum OperatorKind { Other, Bitwise, Logical }; + +// Determine if the operator is a logical or bitwise type. +OperatorKind getOperatorKind(BinaryOperatorKind Opc) { + switch (Opc) { + default: + return Other; + case BO_Or: + case BO_And: + return Bitwise; + case BO_LOr: + case BO_LAnd: + return Logical; + } +} + +// Determine if the operator is a logical or bitwise type. +OperatorKind getOperatorKind(UnaryOperator *UO) { + if (!UO) + return Other; + switch (UO->getOpcode()) { + default: + return Other; + case UO_LNot: + return Logical; + case UO_Not: + return Bitwise; + } +} + +// Switch between bitwise and logical operators. +BinaryOperatorKind getOppositeOpcode(BinaryOperatorKind Opc) { + switch (Opc) { + default: + llvm_unreachable("Unexpected opcode"); + case BO_LAnd: + return BO_And; + case BO_LOr: + return BO_Or; + case BO_And: + return BO_LAnd; + case BO_Or: + return BO_LOr; + } +} + +// Switch between bitwise and logical operators. +UnaryOperatorKind getOppositeOpcode(UnaryOperatorKind Opc) { + switch (Opc) { + default: + llvm_unreachable("Unexpected opcode"); + case UO_LNot: + return UO_Not; + case UO_Not: + return UO_LNot; + } +} + +static void DiagnoseIncorrectNot(Sema &Self, BinaryOperatorKind Opc, + SourceLocation OpLoc, Expr *LHSExpr, + Expr *RHSExpr) { + const OperatorKind BinOpKind = getOperatorKind(Opc); + if (BinOpKind == Other) + return; + + UnaryOperator *LHS_UO = dyn_cast(LHSExpr); + OperatorKind LHSKind = getOperatorKind(LHS_UO); + + UnaryOperator *RHS_UO = dyn_cast(RHSExpr); + OperatorKind RHSKind = getOperatorKind(RHS_UO); + + if (RHSKind == LHSKind) { + // LHS and RHS have same type, possible suggest to change binary operator. + if (RHSKind == Other) { + // No not operators. + return; + } + if (LHSKind == BinOpKind) { + // Operands and binary operator are the same type. + return; + } + + const BinaryOperatorKind NewOpcode = getOppositeOpcode(Opc); + const StringRef NewOperator = BinaryOperator::getOpcodeStr(NewOpcode); + const bool BinaryOrOperator = Opc == BO_Or || Opc == BO_LOr; + + Self.Diag(OpLoc, diag::warn_wrong_binary_operator) + << (BinOpKind == Logical); + Self.Diag(OpLoc, diag::note_suggest_operator) + << (BinOpKind != Logical) << BinaryOrOperator + << FixItHint::CreateReplacement(SourceRange(OpLoc), NewOperator); + return; + } + + if (RHSKind == Other || LHSKind == Other) { + // Either LHS or RHS do not use a not operator. Possible suggest either + // switching the not operator or switching the binary operator. + OperatorKind Kind = (RHSKind != Other) ? RHSKind : LHSKind; + if (Kind == BinOpKind) + return; + + UnaryOperator *NotExpr = (RHSKind != Other) ? RHS_UO : LHS_UO; + + const BinaryOperatorKind NewBinOpcode = getOppositeOpcode(Opc); + const StringRef NewBinaryOperator = + BinaryOperator::getOpcodeStr(NewBinOpcode); + const UnaryOperatorKind NewUnaryOpcode = + getOppositeOpcode(NotExpr->getOpcode()); + const StringRef NewUnaryOperator = + UnaryOperator::getOpcodeStr(NewUnaryOpcode); + const bool BinaryOrOperator = Opc == BO_Or || Opc == BO_LOr; + + Self.Diag(OpLoc, diag::warn_mismatch_operators) << (BinOpKind == Logical) + << BinaryOrOperator + << NotExpr->getExprLoc(); + Self.Diag(OpLoc, diag::note_suggest_operator) + << (BinOpKind == Logical) << BinaryOrOperator + << FixItHint::CreateReplacement(SourceRange(OpLoc), NewBinaryOperator); + Self.Diag(OpLoc, diag::note_suggest_operator) + << (Kind == Logical) << 2 /*not*/ + << FixItHint::CreateReplacement(SourceRange(NotExpr->getOperatorLoc()), + NewUnaryOperator); + return; + } + + if (RHSKind == BinOpKind || LHSKind == BinOpKind) { + // RHS and LHS are different kinds of not operators. Suggest switching one + // of them to match the binary operator. + UnaryOperator *UO = (RHSKind != BinOpKind) ? RHS_UO : LHS_UO; + const bool BinaryOrOperator = Opc == BO_Or || Opc == BO_LOr; + const UnaryOperatorKind NewUnaryOpcode = getOppositeOpcode(UO->getOpcode()); + const StringRef NewUnaryOperator = + UnaryOperator::getOpcodeStr(NewUnaryOpcode); + Self.Diag(OpLoc, diag::warn_mismatch_operators) + << (BinOpKind == Logical) << BinaryOrOperator << UO->getExprLoc(); + Self.Diag(OpLoc, diag::note_suggest_operator) + << (BinOpKind == Logical) << 2 /*not*/ + << FixItHint::CreateReplacement(SourceRange(UO->getOperatorLoc()), + NewUnaryOperator); + return; + } + + llvm_unreachable("unable to process BinaryOperator"); +} +} + // Binary Operators. 'Tok' is the token for the operator. ExprResult Sema::ActOnBinOp(Scope *S, SourceLocation TokLoc, tok::TokenKind Kind, @@ -10108,6 +10256,8 @@ // Emit warnings for tricky precedence issues, e.g. "bitfield & 0x4 == 0" DiagnoseBinOpPrecedence(*this, Opc, TokLoc, LHSExpr, RHSExpr); + DiagnoseIncorrectNot(*this, Opc, TokLoc, LHSExpr, RHSExpr); + return BuildBinOp(S, TokLoc, Opc, LHSExpr, RHSExpr); } Index: test/SemaCXX/warn-mismatch.cpp =================================================================== --- test/SemaCXX/warn-mismatch.cpp +++ test/SemaCXX/warn-mismatch.cpp @@ -0,0 +1,100 @@ +// RUN: %clang_cc1 -fsyntax-only -verify -Wmismatched-operators %s + +void test_negative(int x, int y, int z) { + z = x | y; + z = x & y; + z = ~x | y; + z = ~x & y; + z = x | ~y; + z = x & ~y; + z = ~x | ~y; + z = ~x & ~y; + + z = x || y; + z = x && y; + z = !x || y; + z = !x && y; + z = x || !y; + z = x && !y; + z = !x || !y; + z = !x && !y; +} + +void test_same_operand(int x, int y, int z) { + z = !x | !y; + // expected-warning@-1 {{using bitwise operator when both operands are logical}} + // expected-note@-2 {{did you mean to use logical or?}} + z = !x & !y; + // expected-warning@-1 {{using bitwise operator when both operands are logical}} + // expected-note@-2 {{did you mean to use logical and?}} + z = ~x || ~y; + // expected-warning@-1 {{using logical operator when both operands are bitwise}} + // expected-note@-2 {{did you mean to use bitwise or?}} + z = ~x && ~y; + // expected-warning@-1 {{using logical operator when both operands are bitwise}} + // expected-note@-2 {{did you mean to use bitwise and?}} +} + +void test_different_operand(int x, int y, int z) { + z = !x | ~y; + // expected-warning@-1 {{using bitwise or operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise not?}} + z = !x & ~y; + // expected-warning@-1 {{using bitwise and operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise not?}} + z = !x || ~y; + // expected-warning@-1 {{using logical or operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical not?}} + z = !x && ~y; + // expected-warning@-1 {{using logical and operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical not?}} + + z = ~x | !y; + // expected-warning@-1 {{using bitwise or operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise not?}} + z = ~x & !y; + // expected-warning@-1 {{using bitwise and operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise not?}} + z = ~x || !y; + // expected-warning@-1 {{using logical or operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical not?}} + z = ~x && !y; + // expected-warning@-1 {{using logical and operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical not?}} +} + +void test_one_operand(int x, int y, int z) { + z = !x | y; + // expected-warning@-1 {{using bitwise or operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise or?}} + // expected-note@-3 {{did you mean to use logical not?}} + z = !x & y; + // expected-warning@-1 {{using bitwise and operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise and?}} + // expected-note@-3 {{did you mean to use logical not?}} + z = ~x || y; + // expected-warning@-1 {{using logical or operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical or?}} + // expected-note@-3 {{did you mean to use bitwise not?}} + z = ~x && y; + // expected-warning@-1 {{using logical and operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical and?}} + // expected-note@-3 {{did you mean to use bitwise not?}} + + z = x | !y; + // expected-warning@-1 {{using bitwise or operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise or?}} + // expected-note@-3 {{did you mean to use logical not?}} + z = x & !y; + // expected-warning@-1 {{using bitwise and operator with logical not}} + // expected-note@-2 {{did you mean to use bitwise and?}} + // expected-note@-3 {{did you mean to use logical not?}} + z = x || ~y; + // expected-warning@-1 {{using logical or operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical or?}} + // expected-note@-3 {{did you mean to use bitwise not?}} + z = x && ~y; + // expected-warning@-1 {{using logical and operator with bitwise not}} + // expected-note@-2 {{did you mean to use logical and?}} + // expected-note@-3 {{did you mean to use bitwise not?}} +}