diff mbox series

[C] Do not drop qualifiers for _Atomic in typeof

Message ID 1606161777.5490.5.camel@med.uni-goettingen.de
State New
Headers show
Series [C] Do not drop qualifiers for _Atomic in typeof | expand

Commit Message

Uecker, Martin Nov. 23, 2020, 8:02 p.m. UTC 
Joseph,

here is the patch to not drop qualifiers for _Atomic in
typeof. I am not sure whether this is appropriate in
stage3, but I wanted to leave it here for you to comment
and so that it does not lost.

First, I noticed that the change to drop qualifiers
in lvalue conversion also implies that __auto_type now
always uses the non-qualified type. I think this is more
correct, and also what other compilers and C++'s auto do.
The first change here in c-parser would remove the now
redundant code to drop qualifiers for _Atomic types.

The second change would remove the code to drop qualifiers
for _Atomic types for typeof. I would then use the
comma operator for stdatomic to remove all qualifiers.
Here, the question is whether this may have same
unintended side effects.

--Martin

Comments

Joseph Myers Nov. 23, 2020, 8:21 p.m. UTC | #1 
On Mon, 23 Nov 2020, Uecker, Martin wrote:

> Joseph,
> 
> here is the patch to not drop qualifiers for _Atomic in
> typeof. I am not sure whether this is appropriate in
> stage3, but I wanted to leave it here for you to comment
> and so that it does not lost.
> 
> First, I noticed that the change to drop qualifiers
> in lvalue conversion also implies that __auto_type now
> always uses the non-qualified type. I think this is more
> correct, and also what other compilers and C++'s auto do.
> The first change here in c-parser would remove the now
> redundant code to drop qualifiers for _Atomic types.
> 
> The second change would remove the code to drop qualifiers
> for _Atomic types for typeof. I would then use the
> comma operator for stdatomic to remove all qualifiers.
> Here, the question is whether this may have same
> unintended side effects.

This is OK, with references to bugs 65455 and 92935 as I think it fixes 
those.

Any change to qualifiers for typeof risks breaking something relying on 
the details of when the result is or is not qualified, but given that in 
previous GCC versions that was poorly defined and inconsistent, making 
these changes to make it more consistent seems reasonable.

It is probably still the case that _Typeof as proposed for ISO C would 
need special handling of function and function pointer types for the same 
reason as _Generic has such handling (_Noreturn not being part of the 
function type as defined by ISO C).
Uecker, Martin Nov. 25, 2020, 7:06 p.m. UTC | #2 
Am Montag, den 23.11.2020, 20:21 +0000 schrieb Joseph Myers:
> On Mon, 23 Nov 2020, Uecker, Martin wrote:
> 
> > Joseph,
> > 
> > here is the patch to not drop qualifiers for _Atomic in
> > typeof. I am not sure whether this is appropriate in
> > stage3, but I wanted to leave it here for you to comment
> > and so that it does not lost.
> > 
> > First, I noticed that the change to drop qualifiers
> > in lvalue conversion also implies that __auto_type now
> > always uses the non-qualified type. I think this is more
> > correct, and also what other compilers and C++'s auto do.
> > The first change here in c-parser would remove the now
> > redundant code to drop qualifiers for _Atomic types.
> > 
> > The second change would remove the code to drop qualifiers
> > for _Atomic types for typeof. I would then use the
> > comma operator for stdatomic to remove all qualifiers.
> > Here, the question is whether this may have same
> > unintended side effects.
> 
> This is OK, with references to bugs 65455 and 92935 as I think it fixes 
> those.
> 
> Any change to qualifiers for typeof risks breaking something relying on 
> the details of when the result is or is not qualified, but given that in 
> previous GCC versions that was poorly defined and inconsistent, making 
> these changes to make it more consistent seems reasonable.
> 
> It is probably still the case that _Typeof as proposed for ISO C would 
> need special handling of function and function pointer types for the same 
> reason as _Generic has such handling (_Noreturn not being part of the 
> function type as defined by ISO C).

So OK to apply with the following Changelog?



    C: Do not drop qualifiers in typeof for _Atomic types. [PR65455,PR92935]
    
    2020-11-25  Martin Uecker  <muecker@gwdg.de>

    gcc/c/
            * c-parsers.c (c_parser_declaration_or_fndef): Remove redundant code
	    to drop qualifiers of _Atomic types for __auto_type.
	    (c_parser_typeof_specifier): Do not drop qualifiers of _Atomic
	    types for __typeof__.

    gcc/ginclude/
	    * ginclude/stdatomic.h: Use comma operator to drop qualifiers.
 
    gcc/testsuite/
	    * gcc.dg/typeof-2.c: Adapt test.
Joseph Myers Nov. 25, 2020, 10:27 p.m. UTC | #3 
On Wed, 25 Nov 2020, Uecker, Martin wrote:

> So OK to apply with the following Changelog?

OK fixed as noted.

>     2020-11-25  Martin Uecker  <muecker@gwdg.de>
> 
>     gcc/c/

Should mention the PR number in the ChangeLog entry (the part that will 
end up automatically added to the ChangeLog file), not just the summary 
line.

>             * c-parsers.c (c_parser_declaration_or_fndef): Remove redundant code

It's c-parser.c.  The git hooks will complain if the file names mentioned 
don't match the files changed in the commit.

>     gcc/ginclude/
> 	    * ginclude/stdatomic.h: Use comma operator to drop qualifiers.

gcc/ginclude/ doesn't have its own ChangeLog, this entry goes in gcc/.
diff mbox series

Patch

diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index 7540a15d65d..72dbd2e00c0 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -2224,10 +2224,6 @@  c_parser_declaration_or_fndef (c_parser *parser, bool fndef_ok,
 			      " initializer");
 		  init = convert_lvalue_to_rvalue (init_loc, init, true, true);
 		  tree init_type = TREE_TYPE (init.value);
-		  /* As with typeof, remove all qualifiers from atomic types.  */
-		  if (init_type != error_mark_node && TYPE_ATOMIC (init_type))
-		    init_type
-		      = c_build_qualified_type (init_type, TYPE_UNQUALIFIED);
 		  bool vm_type = variably_modified_type_p (init_type,
 							   NULL_TREE);
 		  if (vm_type)
@@ -3743,11 +3739,6 @@  c_parser_typeof_specifier (c_parser *parser)
       if (was_vm)
 	ret.expr = c_fully_fold (expr.value, false, &ret.expr_const_operands);
       pop_maybe_used (was_vm);
-      /* For use in macros such as those in <stdatomic.h>, remove all
-	 qualifiers from atomic types.  (const can be an issue for more macros
-	 using typeof than just the <stdatomic.h> ones.)  */
-      if (ret.spec != error_mark_node && TYPE_ATOMIC (ret.spec))
-	ret.spec = c_build_qualified_type (ret.spec, TYPE_UNQUALIFIED);
     }
   parens.skip_until_found_close (parser);
   return ret;
diff --git a/gcc/ginclude/stdatomic.h b/gcc/ginclude/stdatomic.h
index b9681640fe1..7c2e08a2c41 100644
--- a/gcc/ginclude/stdatomic.h
+++ b/gcc/ginclude/stdatomic.h
@@ -107,7 +107,7 @@  extern void atomic_signal_fence (memory_order);
 #define ATOMIC_POINTER_LOCK_FREE	__GCC_ATOMIC_POINTER_LOCK_FREE
 
 
-/* Note that these macros require __typeof__ and __auto_type to remove
+/* Note that these macros require __auto_type to remove
    _Atomic qualifiers (and const qualifiers, if those are valid on
    macro operands).
    
@@ -122,7 +122,7 @@  extern void atomic_signal_fence (memory_order);
   __extension__								\
   ({									\
     __auto_type __atomic_store_ptr = (PTR);				\
-    __typeof__ (*__atomic_store_ptr) __atomic_store_tmp = (VAL);	\
+    __typeof__ ((void)0, *__atomic_store_ptr) __atomic_store_tmp = (VAL);	\
     __atomic_store (__atomic_store_ptr, &__atomic_store_tmp, (MO));	\
   })
 
@@ -134,7 +134,7 @@  extern void atomic_signal_fence (memory_order);
   __extension__								\
   ({									\
     __auto_type __atomic_load_ptr = (PTR);				\
-    __typeof__ (*__atomic_load_ptr) __atomic_load_tmp;			\
+    __typeof__ ((void)0, *__atomic_load_ptr) __atomic_load_tmp;			\
     __atomic_load (__atomic_load_ptr, &__atomic_load_tmp, (MO));	\
     __atomic_load_tmp;							\
   })
@@ -146,8 +146,8 @@  extern void atomic_signal_fence (memory_order);
   __extension__								\
   ({									\
     __auto_type __atomic_exchange_ptr = (PTR);				\
-    __typeof__ (*__atomic_exchange_ptr) __atomic_exchange_val = (VAL);	\
-    __typeof__ (*__atomic_exchange_ptr) __atomic_exchange_tmp;		\
+    __typeof__ ((void)0, *__atomic_exchange_ptr) __atomic_exchange_val = (VAL);	\
+    __typeof__ ((void)0, *__atomic_exchange_ptr) __atomic_exchange_tmp;		\
     __atomic_exchange (__atomic_exchange_ptr, &__atomic_exchange_val,	\
 		       &__atomic_exchange_tmp, (MO));			\
     __atomic_exchange_tmp;						\
@@ -161,7 +161,7 @@  extern void atomic_signal_fence (memory_order);
   __extension__								\
   ({									\
     __auto_type __atomic_compare_exchange_ptr = (PTR);			\
-    __typeof__ (*__atomic_compare_exchange_ptr) __atomic_compare_exchange_tmp \
+    __typeof__ ((void)0, *__atomic_compare_exchange_ptr) __atomic_compare_exchange_tmp \
       = (DES);								\
     __atomic_compare_exchange (__atomic_compare_exchange_ptr, (VAL),	\
 			       &__atomic_compare_exchange_tmp, 0,	\
@@ -176,7 +176,7 @@  extern void atomic_signal_fence (memory_order);
   __extension__								\
   ({									\
     __auto_type __atomic_compare_exchange_ptr = (PTR);			\
-    __typeof__ (*__atomic_compare_exchange_ptr) __atomic_compare_exchange_tmp \
+    __typeof__ ((void)0, *__atomic_compare_exchange_ptr) __atomic_compare_exchange_tmp \
       = (DES);								\
     __atomic_compare_exchange (__atomic_compare_exchange_ptr, (VAL),	\
 			       &__atomic_compare_exchange_tmp, 1,	\
diff --git a/gcc/testsuite/gcc.dg/typeof-2.c b/gcc/testsuite/gcc.dg/typeof-2.c
index 21ef5b0b865..68f91c6c361 100644
--- a/gcc/testsuite/gcc.dg/typeof-2.c
+++ b/gcc/testsuite/gcc.dg/typeof-2.c
@@ -1,21 +1,23 @@ 
-/* Test qualifier discard of typeof for atomic types. */
+/* Test qualifier preservation of typeof and discarded for __auto_type. */
 /* { dg-do compile } */
 /* { dg-options "-std=c11" } */
 
-/* Check that the qualifiers are discarded for atomic types. */
+/* Check that the qualifiers are preserved for atomic types. */
 
 extern int i;
 
 extern int * p;
 
 extern int _Atomic const ci;
-extern __typeof (ci) i;
+extern __typeof (ci) ci;
 
 extern int _Atomic volatile vi;
-extern __typeof (vi) i;
+extern __typeof (vi) vi;
 
 extern int * _Atomic restrict ri;
-extern __typeof (ri) p;
+extern __typeof (ri) ri;
+
+/* Check that the qualifiers are discarded for atomic types. */
 
 void f(void)
 {
@@ -46,14 +48,16 @@  extern __typeof (nvi) k;
 extern int * restrict nri;
 extern __typeof (nri) q;
 
+/* Check that the qualifiers are discarded for non-atomic types. */
+
 void g(void)
 {
   __auto_type aci = nci;
-  int const *paci = &aci;
+  int *paci = &aci;
 
   __auto_type avi = nvi;
-  int volatile *pavi = &avi;
+  int *pavi = &avi;
 
   __auto_type ari = nri;
-  int * restrict *pari = &ari;
+  int **pari = &ari;
 }