[clang] 0ada5c7 - [Clang] Separate implicit int conversion on negation sign to new diagnostic group (#139429)

[via cfe-commits]

Author: Yutong Zhu
Date: 2025-06-02T07:27:32-04:00
New Revision: 0ada5c7b1a52afb668bc42dd2d5573e5805433d1

URL: https://github.com/llvm/llvm-project/commit/0ada5c7b1a52afb668bc42dd2d5573e5805433d1
DIFF: https://github.com/llvm/llvm-project/commit/0ada5c7b1a52afb668bc42dd2d5573e5805433d1.diff

LOG: [Clang] Separate implicit int conversion on negation sign to new diagnostic group (#139429)

This PR reverts a change made in #126846. 

#126846 introduced an ``-Wimplicit-int-conversion`` diagnosis for 
```c++
int8_t x = something;
x = -x; // warning here
```

This is technically correct since -x could have a width of 9, but this
pattern is common in codebases.

Reverting this change would also introduce the false positive I fixed in
#126846:
```c++
bool b(signed char c) {
  return -c >= 128; // -c can be 128
}
```

This false positive is uncommon, so I think it makes sense to revert the
change.

Added: 
    clang/test/Sema/implicit-int-conversion-on-int.c

Modified: 
    clang/docs/ReleaseNotes.rst
    clang/include/clang/Basic/DiagnosticGroups.td
    clang/include/clang/Basic/DiagnosticSemaKinds.td
    clang/lib/Sema/SemaChecking.cpp

Removed: 
    


################################################################################
diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index dc97883de05d0..91b89a0946555 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -601,6 +601,10 @@ Improvements to Clang's diagnostics
   trigger a ``'Blue' is deprecated`` warning, which can be turned off with
   ``-Wno-deprecated-declarations-switch-case``.
 
+- Split diagnosis of implicit integer comparison on negation to a new
+  diagnostic group ``-Wimplicit-int-comparison-on-negation``, grouped under
+  ``-Wimplicit-int-conversion``, so user can turn it off independently.
+
 Improvements to Clang's time-trace
 ----------------------------------
 

diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 60c650583801a..be75b9ee6e3f6 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -116,9 +116,11 @@ def DeprecatedOFast : DiagGroup<"deprecated-ofast">;
 def ObjCSignedCharBoolImplicitIntConversion :
   DiagGroup<"objc-signed-char-bool-implicit-int-conversion">;
 def Shorten64To32 : DiagGroup<"shorten-64-to-32">;
+def ImplicitIntConversionOnNegation : DiagGroup<"implicit-int-conversion-on-negation">;
 def ImplicitIntConversion : DiagGroup<"implicit-int-conversion",
                                      [Shorten64To32,
-                                      ObjCSignedCharBoolImplicitIntConversion]>;
+                                      ObjCSignedCharBoolImplicitIntConversion,
+                                      ImplicitIntConversionOnNegation]>;
 def ImplicitConstIntFloatConversion : DiagGroup<"implicit-const-int-float-conversion">;
 def ImplicitIntFloatConversion : DiagGroup<"implicit-int-float-conversion",
  [ImplicitConstIntFloatConversion]>;

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index efc842bb4c42e..6f1e8d9fc74e6 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -4288,6 +4288,9 @@ def warn_impcast_integer_sign_conditional : Warning<
 def warn_impcast_integer_precision : Warning<
   "implicit conversion loses integer precision: %0 to %1">,
   InGroup<ImplicitIntConversion>, DefaultIgnore;
+def warn_impcast_integer_precision_on_negation : Warning<
+  "implicit conversion loses integer precision: %0 to %1 on negation">,
+  InGroup<ImplicitIntConversionOnNegation>, DefaultIgnore;
 def warn_impcast_high_order_zero_bits : Warning<
   "higher order bits are zeroes after implicit conversion">,
   InGroup<ImplicitIntConversion>, DefaultIgnore;

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index aba39c0eb3299..3193359923fdb 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -12304,6 +12304,12 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
     if (SourceMgr.isInSystemMacro(CC))
       return;
 
+    if (const auto *UO = dyn_cast<UnaryOperator>(E)) {
+      if (UO->getOpcode() == UO_Minus)
+        return DiagnoseImpCast(
+            *this, E, T, CC, diag::warn_impcast_integer_precision_on_negation);
+    }
+
     if (TargetRange.Width == 32 && Context.getIntWidth(E->getType()) == 64)
       return DiagnoseImpCast(*this, E, T, CC, diag::warn_impcast_integer_64_32,
                              /* pruneControlFlow */ true);

diff  --git a/clang/test/Sema/implicit-int-conversion-on-int.c b/clang/test/Sema/implicit-int-conversion-on-int.c
new file mode 100644
index 0000000000000..f555893d9e17a
--- /dev/null
+++ b/clang/test/Sema/implicit-int-conversion-on-int.c
@@ -0,0 +1,50 @@
+// RUN: %clang_cc1 %s -verify=expected -Wimplicit-int-conversion
+// RUN: %clang_cc1 %s -verify=none -Wimplicit-int-conversion -Wno-implicit-int-conversion-on-negation
+
+// none-no-diagnostics
+
+char test_char(char x) {
+  return -x; // expected-warning {{implicit conversion loses integer precision: 'int' to 'char' on negation}}
+}
+
+unsigned char test_unsigned_char(unsigned char x) {
+  return -x; // expected-warning {{implicit conversion loses integer precision: 'int' to 'unsigned char' on negation}}
+}
+
+short test_short(short x) {
+  return -x; // expected-warning {{implicit conversion loses integer precision: 'int' to 'short' on negation}}
+}
+
+unsigned short test_unsigned_short(unsigned short x) {
+  return -x; // expected-warning {{implicit conversion loses integer precision: 'int' to 'unsigned short' on negation}}
+}
+
+// --- int-width and wider (should NOT warn) ---
+
+int test_i(int x) {
+  return -x;
+}
+
+unsigned int test_ui(unsigned int x) {
+  return -x;
+}
+
+long test_l(long x) {
+  return -x;
+}
+
+unsigned long test_ul(unsigned long x) {
+  return -x;
+}
+
+long long test_ll(long long x) {
+  return -x;
+}
+
+unsigned long long test_ull(unsigned long long x) {
+  return -x;
+}
+
+unsigned _BitInt(16) test_unsigned_bit_int(unsigned _BitInt(16) x) {
+  return -x;
+}