Re: [PATCH] c: Implement new -Wenum-int-mismatch warning [PR105131]

2022-05-18 Thread Pedro Alves
On 2022-05-18 00:56, Marek Polacek via Gcc-patches wrote:

> +In C, an enumerated type is compatible with @code{char}, a signed
> +integer type, or an unsigned integer type.  However, since the choice
> +of the underlying type of an enumerated type is implementation-defined,
> +such mismatches may cause portability issues.  In C++, such mismatches
> +are an error.  In C, this warning is enabled by @option{-Wall}.

Should it also be enabled by -Wc++-compat?

Pedro Alves


[PATCH] c: Implement new -Wenum-int-mismatch warning [PR105131]

2022-05-17 Thread Marek Polacek via Gcc-patches
In C, an enumerated type is compatible with char, a signed integer type,
or an unsigned integer type (6.7.2.2/5).  Therefore this code compiles:

  enum E { l = -1, z = 0, g = 1 };
  int foo(void);
  enum E foo(void) { return z; }

if the underlying type of 'enum E' is 'int' (if not, we emit an error).
This is different for typedefs, where C11 permits typedefs to be
redeclared to the same type, but not to compatible types.  In C++, the
code above is invalid.

It seems desirable to emit a warning in the C case, because it is
probably a mistake and definitely a portability error, given that the
choice of the underlying type is implementation-defined.

To that end, this patch implements a new -Wenum-int-mismatch warning.
Conveniently, we already have comptypes_check_enum_int to detect such
mismatches.

Bootstrapped/regtested on x86_64-pc-linux-gnu, ok for trunk?

PR c/105131

gcc/c-family/ChangeLog:

* c.opt (Wenum-int-mismatch): New.

gcc/c/ChangeLog:

* c-decl.cc (diagnose_mismatched_decls): Warn about enum/integer type
mismatches.
* c-tree.h (comptypes_check_enum_int): Declare.
* c-typeck.cc (comptypes): No longer static.

gcc/ChangeLog:

* doc/invoke.texi: Document -Wenum-int-mismatch.

gcc/testsuite/ChangeLog:

* gcc.dg/Wenum-int-mismatch-1.c: New test.
* gcc.dg/Wenum-int-mismatch-2.c: New test.
---
 gcc/c-family/c.opt  |  4 +++
 gcc/c/c-decl.cc | 13 ++--
 gcc/c/c-tree.h  |  1 +
 gcc/c/c-typeck.cc   |  2 +-
 gcc/doc/invoke.texi | 20 
 gcc/testsuite/gcc.dg/Wenum-int-mismatch-1.c | 35 +
 gcc/testsuite/gcc.dg/Wenum-int-mismatch-2.c | 35 +
 7 files changed, 107 insertions(+), 3 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/Wenum-int-mismatch-1.c
 create mode 100644 gcc/testsuite/gcc.dg/Wenum-int-mismatch-2.c

diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 035b1de0d84..0cb64283261 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -638,6 +638,10 @@ Wenum-conversion
 C ObjC C++ ObjC++ Var(warn_enum_conversion) Init(0) Warning LangEnabledBy(C 
ObjC,Wextra)
 Warn about implicit conversion of enum types.
 
+Wenum-int-mismatch
+C ObjC Var(warn_enum_int_mismatch) Warning LangEnabledBy(C ObjC,Wall)
+Warn about enum/integer type mismatches.
+
 Werror
 C ObjC C++ ObjC++
 ; Documented in common.opt
diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 83655548fc4..5266a61b859 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -1993,9 +1993,12 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
 
   bool pedwarned = false;
   bool warned = false;
+  bool enum_and_int_p = false;
   auto_diagnostic_group d;
 
-  if (!comptypes (oldtype, newtype))
+  int comptypes_result = comptypes_check_enum_int (oldtype, newtype,
+  _and_int_p);
+  if (!comptypes_result)
 {
   if (TREE_CODE (olddecl) == FUNCTION_DECL
  && fndecl_built_in_p (olddecl, BUILT_IN_NORMAL)
@@ -2137,6 +2140,13 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
  return false;
}
 }
+  /* Warn about enum/integer type mismatches.  They are compatible types
+ (C2X 6.7.2.2/5), but may pose portability problems.  */
+  else if (enum_and_int_p && TREE_CODE (newdecl) != TYPE_DECL)
+warned = warning_at (DECL_SOURCE_LOCATION (newdecl),
+OPT_Wenum_int_mismatch,
+"conflicting types for %q+D due to enum/integer "
+"mismatch; have %qT", newdecl, newtype);
 
   /* Redeclaration of a type is a constraint violation (6.7.2.3p1),
  but silently ignore the redeclaration if either is in a system
@@ -2146,7 +2156,6 @@ diagnose_mismatched_decls (tree newdecl, tree olddecl,
   if (TREE_CODE (newdecl) == TYPE_DECL)
 {
   bool types_different = false;
-  int comptypes_result;
 
   comptypes_result
= comptypes_check_different_types (oldtype, newtype, _different);
diff --git a/gcc/c/c-tree.h b/gcc/c/c-tree.h
index c70f0ba5ab6..2bcb9662620 100644
--- a/gcc/c/c-tree.h
+++ b/gcc/c/c-tree.h
@@ -685,6 +685,7 @@ extern tree require_complete_type (location_t, tree);
 extern bool same_translation_unit_p (const_tree, const_tree);
 extern int comptypes (tree, tree);
 extern int comptypes_check_different_types (tree, tree, bool *);
+extern int comptypes_check_enum_int (tree, tree, bool *);
 extern bool c_vla_type_p (const_tree);
 extern bool c_mark_addressable (tree, bool = false);
 extern void c_incomplete_type_error (location_t, const_tree, const_tree);
diff --git a/gcc/c/c-typeck.cc b/gcc/c/c-typeck.cc
index bcfe08b82bc..4f3611f1b89 100644
--- a/gcc/c/c-typeck.cc
+++ b/gcc/c/c-typeck.cc
@@ -1055,7 +1055,7 @@ comptypes (tree type1, tree type2)
 /* Like comptypes, but if it returns non-zero because enum and int are