[PATCH] D67559: [Sema] Split of versions of -Wimplicit-{float,int}-conversion for Objective-C BOOL

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Mostly LGTM but a few nits.



================
Comment at: clang/include/clang/Basic/DiagnosticGroups.td:65
+def ObjCSignedCharBoolImplicitIntConversion :
+  DiagGroup<"objc-signed-char-bool-implicit-int-conversion">;
+def ImplicitIntConversion : DiagGroup<"implicit-int-conversion",
----------------
This makes me wish that all shells would support autocomplete of program options. :-D


================
Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3312
+def warn_impcast_float_to_objc_signed_char_bool : Warning<
+  "implicit conversion from floating-point type %0 to BOOL">,
+  InGroup<ObjCSignedCharBoolImplicitFloatConversion>;
----------------
Can you put single quotes around `BOOL` since it's a type name? Same below.


================
Comment at: clang/lib/AST/Expr.cpp:191
+
+  if (auto *OVE = dyn_cast<OpaqueValueExpr>(E))
+    return OVE->getSourceExpr()->isKnownToHaveBooleanValue();
----------------
`const auto *` ?


================
Comment at: clang/test/SemaObjC/signed-char-bool-conversion.m:3
+// RUN: %clang_cc1 -xobjective-c++ %s -verify -Wobjc-signed-char-bool
+
+typedef signed char BOOL;
----------------
Can you add a test verifying the fix-it behaviors (with and without parens)?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67559/new/

https://reviews.llvm.org/D67559