[Sema] Split of versions of -Wimplicit-{float,int}-conversion for Obj…

…ective-C BOOL Also, add a diagnostic group, -Wobjc-signed-char-bool, to control all these related diagnostics. rdar://51954400 Differential revision: https://reviews.llvm.org/D67559 git-svn-id: https://llvm.org/svn/llvm-project/cfe/trunk@372183 91177308-0d34-0410-b5e6-96231b3b80d8
cuzdav · Sep 17, 2019 · d8a2a18 · d8a2a18
1 parent 6217ae8
commit d8a2a18
Show file tree

Hide file tree

Showing 6 changed files with 156 additions and 32 deletions.
diff --git a/include/clang/Basic/DiagnosticGroups.td b/include/clang/Basic/DiagnosticGroups.td
@@ -61,9 +61,16 @@ def BoolConversion : DiagGroup<"bool-conversion", [PointerBoolConversion,
                                                    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 @@ def ObjCLiteralComparison : DiagGroup<"objc-literal-compare", [
     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">;

diff --git a/include/clang/Basic/DiagnosticSemaKinds.td b/include/clang/Basic/DiagnosticSemaKinds.td
@@ -3290,7 +3290,7 @@ 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<
+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<ObjCBoolConstantConversion>;
@@ -3308,6 +3308,12 @@ def warn_impcast_literal_float_to_integer_out_of_range : Warning<
 def warn_impcast_float_integer : Warning<
   "implicit conversion turns floating-point number into integer: %0 to %1">,
   InGroup<FloatConversion>, DefaultIgnore;
+def warn_impcast_float_to_objc_signed_char_bool : Warning<
+  "implicit conversion from floating-point type %0 to 'BOOL'">,
+  InGroup<ObjCSignedCharBoolImplicitFloatConversion>;
+def warn_impcast_int_to_objc_signed_char_bool : Warning<
+  "implicit conversion from integral type %0 to 'BOOL'">,
+  InGroup<ObjCSignedCharBoolImplicitIntConversion>, DefaultIgnore;
 
 // Implicit int -> float conversion precision loss warnings.
 def warn_impcast_integer_float_precision : Warning<

diff --git a/lib/AST/Expr.cpp b/lib/AST/Expr.cpp
@@ -185,6 +185,12 @@ bool Expr::isKnownToHaveBooleanValue() const {
     return CO->getTrueExpr()->isKnownToHaveBooleanValue() &&
            CO->getFalseExpr()->isKnownToHaveBooleanValue();
 
+  if (isa<ObjCBoolLiteralExpr>(E))
+    return true;
+
+  if (const auto *OVE = dyn_cast<OpaqueValueExpr>(E))
+    return OVE->getSourceExpr()->isKnownToHaveBooleanValue();
+
   return false;
 }
 

diff --git a/lib/Sema/SemaChecking.cpp b/lib/Sema/SemaChecking.cpp
@@ -10840,6 +10840,26 @@ static void DiagnoseImpCast(Sema &S, Expr *E, QualType T,
   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<OpaqueValueExpr>(Ignored))
+    Ignored = OVE->getSourceExpr();
+  bool NeedsParens = isa<AbstractConditionalOperator>(Ignored) ||
+                     isa<BinaryOperator>(Ignored) ||
+                     isa<CXXOperatorCallExpr>(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 @@ static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
   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 @@ static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
   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 @@ static void DiagnoseFloatingImpCast(Sema &S, Expr *E, QualType T,
     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 @@ 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,
@@ -11254,19 +11283,13 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
   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");
+                         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 @@ static void CheckImplicitConversion(Sema &S, Expr *E, QualType T,
   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);
 

diff --git a/test/Sema/objc-bool-constant-conversion-fixit.m b/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 @@ int main() {
 
   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 @@ void f(BoolProp *bp) {
 
   [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;
 }
diff --git a/test/SemaObjC/signed-char-bool-conversion.m b/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}}
+}