Re: [C PATCH 3/6] c23: tag compatibility rules for struct and unions

[Joseph Myers]

On Sat, 26 Aug 2023, Martin Uecker via Gcc-patches wrote:

>   types (convert_for_assignment): Ingore qualifiers.

"Ignore".

> @@ -1993,6 +1993,24 @@ locate_old_decl (tree decl)
>   decl, TREE_TYPE (decl));
>  }
>  
> +static tree
> +previous_tag (tree type)

This function needs a comment documenting its semantics.

> @@ -8651,6 +8672,12 @@ start_struct (location_t loc, enum tree_code code, 
> tree name,
>  
>if (name != NULL_TREE)
>  ref = lookup_tag (code, name, true, );
> +
> +  /* For C2X, even if we already have a completed definition,
> + we do not use it. We will check for consistency later.  */
> +  if (flag_isoc2x && ref && TYPE_SIZE (ref))
> +ref = NULL_TREE;
> +
>if (ref && TREE_CODE (ref) == code)
>  {
>if (TYPE_STUB_DECL (ref))

This comes before the check for nested redefinitions (which are still 
invalid) - so meaning that, if ref is set to NULL_TREE here, the check 
for nested redefinitions won't apply.

You have a testcase for nested redefinitions in a slightly different case 
(where the struct's first definition hasn't finished when the nested 
definition is encountered).  But what about the case where: first, the 
struct gets defined; then, in the same scope, it gets redefined, with the 
redefinition containing a nested redefinition?  I don't see anything here 
to detect that case of nested redefinitions

For enums, note that nested redefinitions include cases where the nesting 
is inside an enum type specifier (currently diagnosed by GCC following an 
ordinary redefinition path, not one for nested definitions).

typedef __SIZE_TYPE__ size_t;
enum e : typeof (sizeof (enum e : size_t { A })) { A };

is invalid because the definitions of enum e are nested, so should be 
diagnosed, and there should be a test that it is.

> @@ -8315,6 +8332,13 @@ digest_init (location_t init_loc, tree type, tree 
> init, tree origtype,
>  conversion.  */
>   inside_init = convert (type, inside_init);
>  
> +  if ((code == RECORD_TYPE || code == UNION_TYPE)
> +   && !comptypes (TYPE_MAIN_VARIANT (type), TYPE_MAIN_VARIANT (TREE_TYPE 
> (inside_init
> + {
> +   error_init (init_loc, "invalid initializer %qT %qT", type, TREE_TYPE 
> (inside_init));
> +   return error_mark_node;
> + }

I'd expect some words between the two type names, or explaining how they 
relate to the initialization, rather than just two type names in 
succession with no explanation of what's the type of the initializer and 
what's the type of the object being initialized.

> diff --git a/gcc/testsuite/gcc.dg/c2x-tag-1.c 
> b/gcc/testsuite/gcc.dg/c2x-tag-1.c

> +struct r { int a; char b[0]; };

I tend to think tests such as this, involving GNU extensions ([0] arrays), 
should go in gnu23-* tests not c23-* ones.

(I'm currently testing the final C2X -> C23 patch, that renames existing 
tests.  The next revision of this patch series will need updating for the 
renaming in both file names and file contents.)

> +++ b/gcc/testsuite/gcc.dg/c2x-tag-10.c

This is definitely a GNU extensions test (VLAs in structures).

> +++ b/gcc/testsuite/gcc.dg/c2x-tag-4.c

Another GNU extensions test (GNU attributes).

> diff --git a/gcc/testsuite/gcc.dg/c2x-tag-7.c 
> b/gcc/testsuite/gcc.dg/c2x-tag-7.c

Another GNU extensions test (VLAs in structures).

-- 
Joseph S. Myers
jos...@codesourcery.com

[C PATCH 3/6] c23: tag compatibility rules for struct and unions

[Martin Uecker via Gcc-patches]

Implement redeclaration and compatibility rules for
structures and unions in C23.

gcc/c/:
* c-decl.cc (previous_tag): New function.
(get_parm_info): Turn off warning for C2X.
(start_struct): Allow redefinitons.
(finish_struct): Diagnose conflicts.
* c-tree.h (comptypes_same_p): Add prototype.
* c-typeck.cc (comptypes_same_p): New function
(comptypes_internal): Activate comparison of tagged
types (convert_for_assignment): Ingore qualifiers.
(digest_init): Add error.
(initialized_elementwise_p): Allow compatible types.

gcc/testsuite/:
* gcc.dg/c2x-enum-7.c: Remove warning.
* gcc.dg/c2x-tag-1.c: New test.
* gcc.dg/c2x-tag-2.c: New test.
* gcc.dg/c2x-tag-3.c: New test.
* gcc.dg/c2x-tag-4.c: New test.
* gcc.dg/c2x-tag-5.c: New test.
* gcc.dg/c2x-tag-6.c: New test.
* gcc.dg/c2x-tag-7.c: New test.
* gcc.dg/c2x-tag-8.c: New test.
* gcc.dg/c2x-tag-9.c: New test.
* gcc.dg/c2x-tag-10.c: New test.
---
 gcc/c/c-decl.cc   | 56 ++---
 gcc/c/c-tree.h|  1 +
 gcc/c/c-typeck.cc | 38 +
 gcc/testsuite/gcc.dg/c2x-enum-7.c |  6 +--
 gcc/testsuite/gcc.dg/c2x-tag-1.c  | 68 +++
 gcc/testsuite/gcc.dg/c2x-tag-10.c | 31 ++
 gcc/testsuite/gcc.dg/c2x-tag-2.c  | 43 +++
 gcc/testsuite/gcc.dg/c2x-tag-3.c  | 16 
 gcc/testsuite/gcc.dg/c2x-tag-4.c  | 19 +
 gcc/testsuite/gcc.dg/c2x-tag-5.c  | 26 
 gcc/testsuite/gcc.dg/c2x-tag-6.c  | 34 
 gcc/testsuite/gcc.dg/c2x-tag-7.c  | 28 +
 gcc/testsuite/gcc.dg/c2x-tag-8.c  | 25 
 gcc/testsuite/gcc.dg/c2x-tag-9.c  | 12 ++
 14 files changed, 387 insertions(+), 16 deletions(-)
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-1.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-10.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-2.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-3.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-4.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-5.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-6.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-7.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-8.c
 create mode 100644 gcc/testsuite/gcc.dg/c2x-tag-9.c

diff --git a/gcc/c/c-decl.cc b/gcc/c/c-decl.cc
index 1f9eb44dbaa..c5c6a853fa9 100644
--- a/gcc/c/c-decl.cc
+++ b/gcc/c/c-decl.cc
@@ -1993,6 +1993,24 @@ locate_old_decl (tree decl)
decl, TREE_TYPE (decl));
 }
 
+static tree
+previous_tag (tree type)
+{
+  struct c_binding *b = NULL;
+  tree name = TYPE_NAME (type);
+
+  if (name)
+b = I_TAG_BINDING (name);
+
+  if (b)
+b = b->shadowed;
+
+  if (b && B_IN_CURRENT_SCOPE (b))
+return b->decl;
+
+  return NULL_TREE;
+}
+
 /* Subroutine of duplicate_decls.  Compare NEWDECL to OLDDECL.
Returns true if the caller should proceed to merge the two, false
if OLDDECL should simply be discarded.  As a side effect, issues
@@ -8442,11 +8460,14 @@ get_parm_info (bool ellipsis, tree expr)
  if (TREE_CODE (decl) != UNION_TYPE || b->id != NULL_TREE)
{
  if (b->id)
-   /* The %s will be one of 'struct', 'union', or 'enum'.  */
-   warning_at (b->locus, 0,
-   "%<%s %E%> declared inside parameter list"
-   " will not be visible outside of this definition or"
-   " declaration", keyword, b->id);
+   {
+ /* The %s will be one of 'struct', 'union', or 'enum'.  */
+ if (!flag_isoc2x)
+   warning_at (b->locus, 0,
+   "%<%s %E%> declared inside parameter list"
+   " will not be visible outside of this 
definition or"
+   " declaration", keyword, b->id);
+   }
  else
/* The %s will be one of 'struct', 'union', or 'enum'.  */
warning_at (b->locus, 0,
@@ -8651,6 +8672,12 @@ start_struct (location_t loc, enum tree_code code, tree 
name,
 
   if (name != NULL_TREE)
 ref = lookup_tag (code, name, true, );
+
+  /* For C2X, even if we already have a completed definition,
+ we do not use it. We will check for consistency later.  */
+  if (flag_isoc2x && ref && TYPE_SIZE (ref))
+ref = NULL_TREE;
+
   if (ref && TREE_CODE (ref) == code)
 {
   if (TYPE_STUB_DECL (ref))
@@ -9439,6 +9466,25 @@ finish_struct (location_t loc, tree t, tree fieldlist, 
tree attributes,
   warning_at (loc, 0, "union cannot be made transparent");
 }
 
+  /* Check for consistency with previous definition */
+  if (flag_isoc2x)
+{
+  tree vistype = previous_tag (t);
+  if (vistype
+ && TREE_CODE (vistype) == TREE_CODE (t)
+ &&