From abffae3a563d67d0a31a9687aecc4de33a4cc273 Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Tue, 9 Jul 2019 17:29:40 +0000 Subject: [PATCH] [ObjC] Add a warning for implicit conversions of a constant non-boolean value to BOOL rdar://51954400 Differential revision: https://reviews.llvm.org/D63912 llvm-svn: 365518 --- clang/include/clang/Basic/DiagnosticGroups.td | 6 ++-- clang/include/clang/Basic/DiagnosticSemaKinds.td | 4 +++ clang/lib/Sema/SemaChecking.cpp | 28 +++++++++++++++ .../Sema/objc-bool-constant-conversion-fixit.m | 40 ++++++++++++++++++++++ clang/test/Sema/objc-bool-constant-conversion.m | 38 ++++++++++++++++++++ 5 files changed, 114 insertions(+), 2 deletions(-) create mode 100644 clang/test/Sema/objc-bool-constant-conversion-fixit.m create mode 100644 clang/test/Sema/objc-bool-constant-conversion.m diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td index 71500d3..56f2ecfe 100644 --- a/clang/include/clang/Basic/DiagnosticGroups.td +++ b/clang/include/clang/Basic/DiagnosticGroups.td @@ -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">; diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 6eee62d..d59155f 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -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; +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; def warn_impcast_fixed_point_range : Warning< "implicit conversion from %0 cannot fit within the range of values for %1">, diff --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp index 7ab0f2d..64ee7c8 100644 --- a/clang/lib/Sema/SemaChecking.cpp +++ b/clang/lib/Sema/SemaChecking.cpp @@ -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) { @@ -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(Ignored) || + isa(Ignored) || + isa(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 *. if (auto *ArrayLiteral = dyn_cast(E)) diff --git a/clang/test/Sema/objc-bool-constant-conversion-fixit.m b/clang/test/Sema/objc-bool-constant-conversion-fixit.m new file mode 100644 index 0000000..57f5752 --- /dev/null +++ b/clang/test/Sema/objc-bool-constant-conversion-fixit.m @@ -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]; +} diff --git a/clang/test/Sema/objc-bool-constant-conversion.m b/clang/test/Sema/objc-bool-constant-conversion.m new file mode 100644 index 0000000..3638e2f --- /dev/null +++ b/clang/test/Sema/objc-bool-constant-conversion.m @@ -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}} +} -- 2.7.4