Skip to content

Commit

Permalink
[ObjC] Add a warning for implicit conversions of a constant non-boole…
Browse files Browse the repository at this point in the history
…an value to BOOL

rdar://51954400

Differential revision: https://reviews.llvm.org/D63912

git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@365518 91177308-0d34-0410-b5e6-96231b3b80d8
  • Loading branch information
epilk committed Jul 9, 2019
1 parent e79d438 commit 41e6b1c
Show file tree
Hide file tree
Showing 5 changed files with 114 additions and 2 deletions.
6 changes: 4 additions & 2 deletions include/clang/Basic/DiagnosticGroups.td
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,10 @@ def BitFieldWidth : DiagGroup<"bitfield-width">;
def CoroutineMissingUnhandledException :
DiagGroup<"coroutine-missing-unhandled-exception">;
def Coroutine : DiagGroup<"coroutine", [CoroutineMissingUnhandledException]>;
def ConstantConversion :
DiagGroup<"constant-conversion", [ BitFieldConstantConversion ] >;
def ObjCBoolConstantConversion : DiagGroup<"objc-bool-constant-conversion">;
def ConstantConversion : DiagGroup<"constant-conversion",
[BitFieldConstantConversion,
ObjCBoolConstantConversion]>;
def LiteralConversion : DiagGroup<"literal-conversion">;
def StringConversion : DiagGroup<"string-conversion">;
def SignConversion : DiagGroup<"sign-conversion">;
Expand Down
4 changes: 4 additions & 0 deletions include/clang/Basic/DiagnosticSemaKinds.td
Original file line number Diff line number Diff line change
Expand Up @@ -3248,6 +3248,10 @@ def warn_impcast_integer_precision_constant : Warning<
def warn_impcast_bitfield_precision_constant : Warning<
"implicit truncation from %2 to bit-field changes value from %0 to %1">,
InGroup<BitFieldConstantConversion>;
def warn_impcast_constant_int_to_objc_bool : Warning<
"implicit conversion from constant value %0 to BOOL; "
"the only well defined values for BOOL are YES and NO">,
InGroup<ObjCBoolConstantConversion>;

def warn_impcast_fixed_point_range : Warning<
"implicit conversion from %0 cannot fit within the range of values for %1">,
Expand Down
28 changes: 28 additions & 0 deletions lib/Sema/SemaChecking.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -11149,6 +11149,11 @@ static bool isSameWidthConstantConversion(Sema &S, Expr *E, QualType T,
return true;
}

static bool isObjCSignedCharBool(Sema &S, QualType Ty) {
return Ty->isSpecificBuiltinType(BuiltinType::SChar) &&
S.getLangOpts().ObjC && S.NSAPIObj->isObjCBOOLType(Ty);
}

static void
CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
bool *ICContext = nullptr) {
Expand Down Expand Up @@ -11192,6 +11197,29 @@ CheckImplicitConversion(Sema &S, Expr *E, QualType T, SourceLocation CC,
}
}

// If the we're converting a constant to an ObjC BOOL on a platform where BOOL
// is a typedef for signed char (macOS), then that constant value has to be 1
// or 0.
if (isObjCSignedCharBool(S, T) && Source->isIntegralType(S.Context)) {
Expr::EvalResult Result;
if (E->EvaluateAsInt(Result, S.getASTContext(),
Expr::SE_AllowSideEffects) &&
Result.Val.getInt() != 1 && Result.Val.getInt() != 0) {
auto Builder = S.Diag(CC, diag::warn_impcast_constant_int_to_objc_bool)
<< Result.Val.getInt().toString(10);
Expr *Ignored = E->IgnoreImplicit();
bool NeedsParens = isa<AbstractConditionalOperator>(Ignored) ||
isa<BinaryOperator>(Ignored) ||
isa<CXXOperatorCallExpr>(Ignored);
SourceLocation EndLoc = S.getLocForEndOfToken(E->getEndLoc());
if (NeedsParens)
Builder << FixItHint::CreateInsertion(E->getBeginLoc(), "(")
<< FixItHint::CreateInsertion(EndLoc, ")");
Builder << FixItHint::CreateInsertion(EndLoc, " ? YES : NO");
return;
}
}

// Check implicit casts from Objective-C collection literals to specialized
// collection types, e.g., NSArray<NSString *> *.
if (auto *ArrayLiteral = dyn_cast<ObjCArrayLiteral>(E))
Expand Down
40 changes: 40 additions & 0 deletions test/Sema/objc-bool-constant-conversion-fixit.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
// RUN: %clang_cc1 -Werror=constant-conversion %s -fixit-recompile -fixit-to-temporary -E -o - | FileCheck %s

typedef signed char BOOL;

BOOL b;

int main() {
BOOL b = 2;
// CHECK: BOOL b = 2 ? YES : NO;

b = b ? 2 : 1;
// CHECK: b = b ? 2 ? YES : NO : 1;

b = b ? 1 : 2;
// CHECK: b = b ? 1 : 2 ? YES : NO;

b = b ? 2 : 2;
// CHECK: b = b ? 2 ? YES : NO : 2 ? YES : NO;

b = 1 + 1;
// CHECK: b = (1 + 1) ? YES : NO;

b = 1 | 2;
// CHECK: b = (1 | 2) ? YES : NO;

b = 1 << 1;
// CHECK: b = (1 << 1) ? YES : NO;
}

@interface BoolProp
@property BOOL b;
@end

void f(BoolProp *bp) {
bp.b = 43;
// CHECK: bp.b = 43 ? YES : NO;

[bp setB:43];
// CHECK: [bp setB:43 ? YES : NO];
}
38 changes: 38 additions & 0 deletions test/Sema/objc-bool-constant-conversion.m
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
// RUN: %clang_cc1 %s -verify -fsyntax-only

typedef signed char BOOL;
#define YES __objc_yes
#define NO __objc_no

BOOL B;

int main() {
B = 0;
B = 1;
B = YES;
B = NO;

B = -1; // expected-warning{{implicit conversion from constant value -1 to BOOL; the only well defined values for BOOL are YES and NO}}
B = 0 - 1; // expected-warning{{implicit conversion from constant value -1 to BOOL; the only well defined values for BOOL are YES and NO}}
B = YES + YES; // expected-warning {{implicit conversion from constant value 2 to BOOL; the only well defined values for BOOL are YES and NO}}
B = YES | YES;

B = B ? 2 : 2; // expected-warning 2 {{implicit conversion from constant value 2 to BOOL; the only well defined values for BOOL are YES and NO}}

BOOL Init = -1; // expected-warning{{implicit conversion from constant value -1 to BOOL; the only well defined values for BOOL are YES and NO}}
BOOL Init2 = B ? 2 : 2; // expected-warning 2 {{implicit conversion from constant value 2 to BOOL; the only well defined values for BOOL are YES and NO}}

void takesbool(BOOL);
takesbool(43); // expected-warning {{implicit conversion from constant value 43 to BOOL; the only well defined values for BOOL are YES and NO}}

BOOL OutOfRange = 400; // expected-warning{{implicit conversion from constant value 400 to BOOL; the only well defined values for BOOL are YES and NO}}
}

@interface BoolProp
@property BOOL b;
@end

void f(BoolProp *bp) {
bp.b = 43; // expected-warning {{implicit conversion from constant value 43 to BOOL; the only well defined values for BOOL are YES and NO}}
[bp setB:43]; // expected-warning {{implicit conversion from constant value 43 to BOOL; the only well defined values for BOOL are YES and NO}}
}

0 comments on commit 41e6b1c

Please sign in to comment.