Index: clang/include/clang/Basic/DiagnosticGroups.td =================================================================== --- clang/include/clang/Basic/DiagnosticGroups.td +++ clang/include/clang/Basic/DiagnosticGroups.td @@ -61,9 +61,16 @@ UndefinedBoolConversion]>; def IntConversion : DiagGroup<"int-conversion">; def EnumConversion : DiagGroup<"enum-conversion">; -def ImplicitIntConversion : DiagGroup<"implicit-int-conversion">; +def ObjCSignedCharBoolImplicitIntConversion : + DiagGroup<"objc-signed-char-bool-implicit-int-conversion">; +def ImplicitIntConversion : DiagGroup<"implicit-int-conversion", + [ObjCSignedCharBoolImplicitIntConversion]>; def ImplicitIntFloatConversion : DiagGroup<"implicit-int-float-conversion">; -def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion", [ImplicitIntFloatConversion]>; +def ObjCSignedCharBoolImplicitFloatConversion : + DiagGroup<"objc-signed-char-bool-implicit-float-conversion">; +def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion", + [ImplicitIntFloatConversion, + ObjCSignedCharBoolImplicitFloatConversion]>; def ImplicitFixedPointConversion : DiagGroup<"implicit-fixed-point-conversion">; def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">; @@ -1015,6 +1022,12 @@ ObjCStringComparison ]>; +def ObjCSignedCharBool : DiagGroup<"objc-signed-char-bool", + [ObjCSignedCharBoolImplicitIntConversion, + ObjCSignedCharBoolImplicitFloatConversion, + ObjCBoolConstantConversion, + TautologicalObjCBoolCompare]>; + // Inline ASM warnings. def ASMOperandWidths : DiagGroup<"asm-operand-widths">; def ASMIgnoredQualifier : DiagGroup<"asm-ignored-qualifier">; Index: clang/include/clang/Basic/DiagnosticSemaKinds.td =================================================================== --- clang/include/clang/Basic/DiagnosticSemaKinds.td +++ clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -3290,7 +3290,7 @@ 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< +def warn_impcast_constant_value_to_objc_bool : Warning< "implicit conversion from constant value %0 to BOOL; " "the only well defined values for BOOL are YES and NO">, InGroup; @@ -3308,6 +3308,12 @@ def warn_impcast_float_integer : Warning< "implicit conversion turns floating-point number into integer: %0 to %1">, InGroup, DefaultIgnore; +def warn_impcast_float_to_objc_signed_char_bool : Warning< + "implicit conversion from floating-point type %0 to 'BOOL'">, + InGroup; +def warn_impcast_int_to_objc_signed_char_bool : Warning< + "implicit conversion from integral type %0 to 'BOOL'">, + InGroup, DefaultIgnore; // Implicit int -> float conversion precision loss warnings. def warn_impcast_integer_float_precision : Warning< Index: clang/lib/AST/Expr.cpp =================================================================== --- clang/lib/AST/Expr.cpp +++ clang/lib/AST/Expr.cpp @@ -185,6 +185,12 @@ return CO->getTrueExpr()->isKnownToHaveBooleanValue() && CO->getFalseExpr()->isKnownToHaveBooleanValue(); + if (isa(E)) + return true; + + if (const auto *OVE = dyn_cast(E)) + return OVE->getSourceExpr()->isKnownToHaveBooleanValue(); + return false; } Index: clang/lib/Sema/SemaChecking.cpp =================================================================== --- clang/lib/Sema/SemaChecking.cpp +++ clang/lib/Sema/SemaChecking.cpp @@ -10840,6 +10840,26 @@ DiagnoseImpCast(S, E, E->getType(), T, CContext, diag, pruneControlFlow); } +static bool isObjCSignedCharBool(Sema &S, QualType Ty) { + return Ty->isSpecificBuiltinType(BuiltinType::SChar) && + S.getLangOpts().ObjC && S.NSAPIObj->isObjCBOOLType(Ty); +} + +static void adornObjCBoolConversionDiagWithTernaryFixit( + Sema &S, Expr *SourceExpr, const Sema::SemaDiagnosticBuilder &Builder) { + Expr *Ignored = SourceExpr->IgnoreImplicit(); + if (const auto *OVE = dyn_cast(Ignored)) + Ignored = OVE->getSourceExpr(); + bool NeedsParens = isa(Ignored) || + isa(Ignored) || + isa(Ignored); + SourceLocation EndLoc = S.getLocForEndOfToken(SourceExpr->getEndLoc()); + if (NeedsParens) + Builder << FixItHint::CreateInsertion(SourceExpr->getBeginLoc(), "(") + << FixItHint::CreateInsertion(EndLoc, ")"); + Builder << FixItHint::CreateInsertion(EndLoc, " ? YES : NO"); +} + /// Diagnose an implicit cast from a floating point value to an integer value. static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T, SourceLocation CContext) { @@ -10859,6 +10879,13 @@ bool IsConstant = E->EvaluateAsFloat(Value, S.Context, Expr::SE_AllowSideEffects); if (!IsConstant) { + if (isObjCSignedCharBool(S, T)) { + return adornObjCBoolConversionDiagWithTernaryFixit( + S, E, + S.Diag(CContext, diag::warn_impcast_float_to_objc_signed_char_bool) + << E->getType()); + } + return DiagnoseImpCast(S, E, T, CContext, diag::warn_impcast_float_integer, PruneWarnings); } @@ -10870,6 +10897,23 @@ llvm::APFloat::opStatus Result = Value.convertToInteger( IntegerValue, llvm::APFloat::rmTowardZero, &isExact); + // FIXME: Force the precision of the source value down so we don't print + // digits which are usually useless (we don't really care here if we + // truncate a digit by accident in edge cases). Ideally, APFloat::toString + // would automatically print the shortest representation, but it's a bit + // tricky to implement. + SmallString<16> PrettySourceValue; + unsigned precision = llvm::APFloat::semanticsPrecision(Value.getSemantics()); + precision = (precision * 59 + 195) / 196; + Value.toString(PrettySourceValue, precision); + + if (isObjCSignedCharBool(S, T) && IntegerValue != 0 && IntegerValue != 1) { + return adornObjCBoolConversionDiagWithTernaryFixit( + S, E, + S.Diag(CContext, diag::warn_impcast_constant_value_to_objc_bool) + << PrettySourceValue); + } + if (Result == llvm::APFloat::opOK && isExact) { if (IsLiteral) return; return DiagnoseImpCast(S, E, T, CContext, diag::warn_impcast_float_integer, @@ -10913,16 +10957,6 @@ DiagID = diag::warn_impcast_float_to_integer; } - // FIXME: Force the precision of the source value down so we don't print - // digits which are usually useless (we don't really care here if we - // truncate a digit by accident in edge cases). Ideally, APFloat::toString - // would automatically print the shortest representation, but it's a bit - // tricky to implement. - SmallString<16> PrettySourceValue; - unsigned precision = llvm::APFloat::semanticsPrecision(Value.getSemantics()); - precision = (precision * 59 + 195) / 196; - Value.toString(PrettySourceValue, precision); - SmallString<16> PrettyTargetValue; if (IsBool) PrettyTargetValue = Value.isZero() ? "false" : "true"; @@ -11199,11 +11233,6 @@ 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, @@ -11254,19 +11283,13 @@ 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"); + Expr::SE_AllowSideEffects)) { + if (Result.Val.getInt() != 1 && Result.Val.getInt() != 0) { + adornObjCBoolConversionDiagWithTernaryFixit( + S, E, + S.Diag(CC, diag::warn_impcast_constant_value_to_objc_bool) + << Result.Val.getInt().toString(10)); + } return; } } @@ -11509,6 +11532,14 @@ if (Target->isSpecificBuiltinType(BuiltinType::Bool)) return; + if (isObjCSignedCharBool(S, T) && !Source->isCharType() && + !E->isKnownToHaveBooleanValue()) { + return adornObjCBoolConversionDiagWithTernaryFixit( + S, E, + S.Diag(CC, diag::warn_impcast_int_to_objc_signed_char_bool) + << E->getType()); + } + IntRange SourceRange = GetExprRange(S.Context, E, S.isConstantEvaluated()); IntRange TargetRange = IntRange::forTargetOfCanonicalType(S.Context, Target); Index: clang/test/Sema/objc-bool-constant-conversion-fixit.m =================================================================== --- clang/test/Sema/objc-bool-constant-conversion-fixit.m +++ clang/test/Sema/objc-bool-constant-conversion-fixit.m @@ -1,4 +1,4 @@ -// RUN: %clang_cc1 -Werror=constant-conversion %s -fixit-recompile -fixit-to-temporary -E -o - | FileCheck %s +// RUN: %clang_cc1 -Werror=objc-signed-char-bool %s -fixit-recompile -fixit-to-temporary -E -o - | FileCheck %s typedef signed char BOOL; @@ -25,6 +25,17 @@ b = 1 << 1; // CHECK: b = (1 << 1) ? YES : NO; + + int i; + + b = i; + // CHECK: b = i ? YES : NO; + + b = i * 2; + // CHECK b = (i * 2) ? YES : NO; + + b = 1 ? 2 : 3; + // CHECK: b = 1 ? 2 ? YES : NO : 3 ? YES : NO; } @interface BoolProp @@ -37,4 +48,12 @@ [bp setB:43]; // CHECK: [bp setB:43 ? YES : NO]; + + int i; + + bp.b = i; + // CHECK: bp.b = i ? YES : NO; + + bp.b = i + 1; + // CHECK: bp.b = (i + 1) ? YES : NO; } Index: clang/test/SemaObjC/signed-char-bool-conversion.m =================================================================== --- /dev/null +++ clang/test/SemaObjC/signed-char-bool-conversion.m @@ -0,0 +1,49 @@ +// RUN: %clang_cc1 %s -verify -Wobjc-signed-char-bool +// RUN: %clang_cc1 -xobjective-c++ %s -verify -Wobjc-signed-char-bool + +typedef signed char BOOL; +#define YES __objc_yes +#define NO __objc_no + +typedef unsigned char Boolean; + +BOOL b; +Boolean boolean; +float fl; +int i; +int *ptr; + +void t1() { + b = boolean; + b = fl; // expected-warning {{implicit conversion from floating-point type 'float' to 'BOOL'}} + b = i; // expected-warning {{implicit conversion from integral type 'int' to 'BOOL'}} + + b = 1.0; + b = 0.0; + b = 1.1; // expected-warning {{implicit conversion from 'double' to 'BOOL' (aka 'signed char') changes value from 1.1 to 1}} + b = 2.1; // expected-warning {{implicit conversion from constant value 2.1 to BOOL; the only well defined values for BOOL are YES and NO}} + + b = YES; +#ifndef __cplusplus + b = ptr; // expected-warning {{incompatible pointer to integer conversion assigning to 'BOOL' (aka 'signed char') from 'int *'}} +#endif +} + +@interface BoolProp +@property BOOL p; +@end + +void t2(BoolProp *bp) { + bp.p = YES; + bp.p = NO; + bp.p = boolean; + bp.p = fl; // expected-warning {{implicit conversion from floating-point type 'float' to 'BOOL'}} + bp.p = i; // expected-warning {{implicit conversion from integral type 'int' to 'BOOL'}} + bp.p = b; + bp.p = bp.p; +#ifndef __cplusplus + bp.p = ptr; // expected-warning {{incompatible pointer to integer conversion assigning to 'BOOL' (aka 'signed char') from 'int *'}} +#endif + bp.p = 1; + bp.p = 2; // expected-warning {{implicit conversion from constant value 2 to BOOL; the only well defined values for BOOL are YES and NO}} +}