refactor OBT diagnostic groups and naming

Use -Wconversion diagnostic group patterns instead of hand-rolled ones.

Signed-off-by: Justin Stitt <[email protected]>

Author: JustinStitt

clang/docs/OverflowBehaviorTypes.rst

@@ -161,10 +161,10 @@ Diagnostics
 
 Clang provides diagnostics to help developers manage overflow behavior types.
 
--Wimplicitly-discarded-overflow-behavior
-----------------------------------------
+-Woverflow-behavior-conversion
+------------------------------
 
-This warning is issued when an overflow behavior type is implicitly converted
+This warning group is issued when an overflow behavior type is implicitly converted
 to a standard integer type, which may lead to the loss of the specified
 overflow behavior.
 
@@ -192,39 +192,20 @@ integer type.
     some_function(static_cast<int>(w)); // OK
   }
 
-This warning acts as a group that includes
-``-Wimplicitly-discarded-overflow-behavior-pedantic`` and
-``-Wimplicitly-discarded-overflow-behavior-assignment``.
-
--Wimplicitly-discarded-overflow-behavior-pedantic
--------------------------------------------------
-
-A less severe version of the warning, ``-Wimplicitly-discarded-overflow-behavior-pedantic``,
-is issued for implicit conversions from an unsigned wrapping type to a standard
-unsigned integer type. This is considered less problematic because both types
-have well-defined wrapping behavior, but the conversion still discards the
-explicit ``overflow_behavior`` attribute.
-
-.. code-block:: c++
-
-  typedef unsigned int __attribute__((overflow_behavior(wrap))) wrapping_uint;
-
-  void some_function(unsigned int);
+This warning group includes
+``-Wimplicit-overflow-behavior-conversion`` and
+``-Wimplicit-overflow-behavior-conversion-pedantic``.
 
-  void another_function(wrapping_uint w) {
-    some_function(w); // warning: implicit conversion from 'wrapping_uint' to
-                      // 'unsigned int' discards overflow behavior
-                      // [-Wimplicitly-discarded-overflow-behavior-pedantic]
-  }
+.. note::
+   ``-Woverflow-behavior-conversion`` is implied by ``-Wconversion``.
 
--Wimplicitly-discarded-overflow-behavior-assignment
----------------------------------------------------
+-Wimplicit-overflow-behavior-conversion
+---------------------------------------
 
 This warning is issued when an overflow behavior type is implicitly converted
-to a standard integer type as part of an assignment, which may lead to the
-loss of the specified overflow behavior. This is a more specific version of
-the ``-Wimplicitly-discarded-overflow-behavior`` warning, and it is off by
-default.
+to a standard integer type as part of most conversions, which may lead to the
+loss of the specified overflow behavior. This is the main warning in the
+``-Woverflow-behavior-conversion`` group.
 
 .. code-block:: c++
 
@@ -233,8 +214,23 @@ default.
   void some_function() {
     wrapping_int w = 1;
     int i = w; // warning: implicit conversion from 'wrapping_int' to 'int'
-               // discards overflow behavior
-               // [-Wimplicitly-discarded-overflow-behavior-assignment]
+               // during assignment discards overflow behavior
+               // [-Wimplicit-overflow-behavior-conversion]
+  }
+
+Here's another example showing function parameter conversion with a ``no_wrap`` type:
+
+.. code-block:: c++
+
+  typedef int __attribute__((overflow_behavior(no_wrap))) safe_int;
+
+  void bar(int x); // Function expects standard int
+
+  void foo() {
+    safe_int s = 42;
+    bar(s); // warning: implicit conversion from 'safe_int' to 'int'
+                      // discards overflow behavior
+                      // [-Wimplicit-overflow-behavior-conversion]
   }
 
 To fix this, you can explicitly cast the overflow behavior type to a standard
@@ -243,13 +239,42 @@ integer type.
 .. code-block:: c++
 
   typedef int __attribute__((overflow_behavior(wrap))) wrapping_int;
+  typedef int __attribute__((overflow_behavior(no_wrap))) safe_int;
 
   void some_function() {
     wrapping_int w = 1;
     int i = static_cast<int>(w); // OK
     int j = (int)w; // C-style OK
   }
 
+  void bar(int x);
+
+  void foo() {
+    safe_int s = 42;
+    bar(static_cast<int>(s)); // OK
+  }
+
+
+-Wimplicit-overflow-behavior-conversion-pedantic
+------------------------------------------------
+
+A less severe version of the warning, ``-Wimplicit-overflow-behavior-conversion-pedantic``,
+is issued for implicit conversions from an unsigned wrapping type to a standard
+unsigned integer type. This is considered less problematic because both types
+have well-defined wrapping behavior, but the conversion still discards the
+explicit ``overflow_behavior`` attribute.
+
+.. code-block:: c++
+
+  typedef unsigned int __attribute__((overflow_behavior(wrap))) wrapping_uint;
+
+  void some_function(unsigned int);
+
+  void another_function(wrapping_uint w) {
+    some_function(w); // warning: implicit conversion from 'wrapping_uint' to
+                      // 'unsigned int' discards overflow behavior
+                      // [-Wimplicit-overflow-behavior-conversion-pedantic]
+  }
 
 -Woverflow-behavior-attribute-ignored
 -------------------------------------

clang/include/clang/Basic/DiagnosticGroups.td

@@ -117,18 +117,11 @@ 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,
+def ImplicitIntConversion
+    : DiagGroup<
+          "implicit-int-conversion", [Shorten64To32,
                                       ObjCSignedCharBoolImplicitIntConversion,
                                       ImplicitIntConversionOnNegation]>;
-def ImplicitlyDiscardedOverflowBehaviorPedantic
-    : DiagGroup<"implicitly-discarded-overflow-behavior-pedantic">;
-def ImplicitlyDiscardedOverflowBehaviorAssignment
-    : DiagGroup<"implicitly-discarded-overflow-behavior-assignment">;
-def ImplicitlyDiscardedOverflowBehavior
-    : DiagGroup<"implicitly-discarded-overflow-behavior",
-                [ImplicitlyDiscardedOverflowBehaviorPedantic,
-                 ImplicitlyDiscardedOverflowBehaviorAssignment]>;
 def ImplicitConstIntFloatConversion : DiagGroup<"implicit-const-int-float-conversion">;
 def ImplicitIntFloatConversion : DiagGroup<"implicit-int-float-conversion",
  [ImplicitConstIntFloatConversion]>;
@@ -138,6 +131,14 @@ def ImplicitFloatConversion : DiagGroup<"implicit-float-conversion",
   [ImplicitIntFloatConversion,
    ObjCSignedCharBoolImplicitFloatConversion]>;
 def ImplicitFixedPointConversion : DiagGroup<"implicit-fixed-point-conversion">;
+def ImplicitOverflowBehaviorConversion
+    : DiagGroup<"implicit-overflow-behavior-conversion">;
+def ImplicitOverflowBehaviorConversionPedantic
+    : DiagGroup<"implicit-overflow-behavior-conversion-pedantic">;
+def OverflowBehaviorConversion
+    : DiagGroup<"overflow-behavior-conversion",
+                [ImplicitOverflowBehaviorConversion,
+                 ImplicitOverflowBehaviorConversionPedantic]>;
 
 def FloatOverflowConversion : DiagGroup<"float-overflow-conversion">;
 def FloatZeroConversion : DiagGroup<"float-zero-conversion">;
@@ -1145,23 +1146,16 @@ def Parentheses : DiagGroup<"parentheses",
 //   - conversion warnings for literals are on by default
 //   - bool-to-pointer conversion warnings are on by default
 //   - __null-to-integer conversion warnings are on by default
-def Conversion : DiagGroup<"conversion",
-                           [BoolConversion,
-                            CharacterConversion,
-                            ConstantConversion,
-                            EnumConversion,
-                            BitFieldEnumConversion,
-                            FloatConversion,
-                            IntConversion,
-                            ImplicitIntConversion,
-                            ImplicitFloatConversion,
-                            LiteralConversion,
-                            NonLiteralNullConversion, // (1-1)->pointer (etc)
-                            NullConversion, // NULL->non-pointer
-                            ObjCLiteralConversion,
-                            SignConversion,
-                            StringConversion]>,
-                 DiagCategory<"Value Conversion Issue">;
+def Conversion
+    : DiagGroup<"conversion",
+                [BoolConversion, CharacterConversion, ConstantConversion,
+                 EnumConversion, BitFieldEnumConversion, FloatConversion,
+                 IntConversion, ImplicitIntConversion, ImplicitFloatConversion,
+                 OverflowBehaviorConversion, LiteralConversion,
+                 NonLiteralNullConversion, // (1-1)->pointer (etc)
+                 NullConversion,           // NULL->non-pointer
+                 ObjCLiteralConversion, SignConversion, StringConversion]>,
+      DiagCategory<"Value Conversion Issue">;
 
 def Unused : DiagGroup<"unused",
                        [UnusedArgument, UnusedFunction, UnusedLabel,

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -4043,16 +4043,14 @@ def warn_overflow_behavior_attribute_disabled
     : Warning<"%0 attribute is ignored because it is not enabled; pass "
               "-foverflow-behavior-types">,
       InGroup<OverflowBehaviorAttributeIgnored>;
-def warn_implicitly_discarded_overflow_behavior
-    : Warning<"implicit conversion from %0 to %1 discards overflow behavior">,
-      InGroup<ImplicitlyDiscardedOverflowBehavior>;
-def warn_implicitly_discarded_overflow_behavior_pedantic
-    : Warning<"implicit conversion from %0 to %1 discards overflow behavior">,
-      InGroup<ImplicitlyDiscardedOverflowBehaviorPedantic>;
-def warn_implicitly_discarded_overflow_behavior_assignment
+def warn_impcast_overflow_behavior_assignment
     : Warning<"implicit conversion from %0 to %1 during assignment discards "
               "overflow behavior">,
-      InGroup<ImplicitlyDiscardedOverflowBehaviorAssignment>,
+      InGroup<ImplicitOverflowBehaviorConversion>,
+      DefaultIgnore;
+def warn_impcast_overflow_behavior_pedantic
+    : Warning<"implicit conversion from %0 to %1 discards overflow behavior">,
+      InGroup<ImplicitOverflowBehaviorConversionPedantic>,
       DefaultIgnore;
 // Availability attribute
 def warn_availability_unknown_platform : Warning<
@@ -4335,6 +4333,10 @@ def warn_impcast_vector_scalar : Warning<
 def warn_impcast_complex_scalar : Warning<
   "implicit conversion discards imaginary component: %0 to %1">,
   InGroup<Conversion>, DefaultIgnore;
+def warn_impcast_overflow_behavior
+    : Warning<"implicit conversion from %0 to %1 discards overflow behavior">,
+      InGroup<ImplicitOverflowBehaviorConversion>,
+      DefaultIgnore;
 def err_impcast_complex_scalar : Error<
   "implicit conversion from %0 to %1 is not permitted in C++">;
 def warn_impcast_float_precision : Warning<

clang/lib/Sema/SemaChecking.cpp

@@ -12153,17 +12153,15 @@ void Sema::CheckImplicitConversion(Expr *E, QualType T, SourceLocation CC,
     if (Target->isIntegerType() && !Target->isOverflowBehaviorType()) {
       // Implicit casts from unsigned wrap types to unsigned types are less
       // problematic but still warrant some diagnostic.
-      if (DiscardedDuringAssignment)
-        return DiagnoseImpCast(
-            *this, E, T, CC,
-            diag::warn_implicitly_discarded_overflow_behavior_assignment);
       if (OBT->isUnsignedIntegerType() && OBT->isWrapKind() &&
           Target->isUnsignedIntegerType())
-        return DiagnoseImpCast(
-            *this, E, T, CC,
-            diag::warn_implicitly_discarded_overflow_behavior_pedantic);
+        return DiagnoseImpCast(*this, E, T, CC,
+                               diag::warn_impcast_overflow_behavior_pedantic);
+      if (DiscardedDuringAssignment)
+        return DiagnoseImpCast(*this, E, T, CC,
+                               diag::warn_impcast_overflow_behavior_assignment);
       return DiagnoseImpCast(*this, E, T, CC,
-                             diag::warn_implicitly_discarded_overflow_behavior);
+                             diag::warn_impcast_overflow_behavior);
     }
   }
 

clang/test/Sema/attr-overflow-behavior.c

@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 %s -Winteger-overflow -Wno-unused-value -foverflow-behavior-types -Wimplicitly-discarded-overflow-behavior -verify -fsyntax-only
+// RUN: %clang_cc1 %s -Winteger-overflow -Wno-unused-value -foverflow-behavior-types -Woverflow-behavior-conversion -verify -fsyntax-only
 
 typedef int __attribute__((overflow_behavior)) bad_arg_count; // expected-error {{'overflow_behavior' attribute takes one argument}}
 typedef int __attribute__((overflow_behavior(not_real))) bad_arg_spec; // expected-error {{'not_real' is not a valid argument to attribute 'overflow_behavior'}}
@@ -34,7 +34,7 @@ void imp_disc_test(unsigned __attribute__((overflow_behavior(wrap))) a) {
   imp_disc(a); // expected-warning {{implicit conversion from '__wrap unsigned int' to 'int' discards overflow behavior}}
 }
 
-// -Wimplicitly-discarded-overflow-behavior-assignment
+// -Wconversion for assignments that discard overflow behavior
 void assignment_disc_test(unsigned __attribute__((overflow_behavior(wrap))) a) {
   int b = a; // expected-warning {{implicit conversion from '__wrap unsigned int' to 'int' during assignment discards overflow behavior}}
   int c = (int)a; // OK