diff mbox

[RFC] Variants of __typeof

Message ID 56B2E940.5060108@redhat.com
State New
Headers show

Commit Message

Richard Henderson Feb. 4, 2016, 6:01 a.m. UTC
While attempting to write some code that uses the new x86 named address space 
support in gcc 6, I found that __typeof is very unhelpful.  In particular, given

	int __seg_fs *ptr;
	__typeof(*ptr) obj;

OBJ will not be type "int", but "int __seg_fs".  Which means that you can't use 
it to create temporaries within statement expressions.

In the process of writing this, I found a hack in __typeof added just to 
support _Atomic.  Which suggests that one of these variants would be more 
generally helpful than the hack.

I add __typeof_noas and __typeof_noqual.  The first strips only the address 
space, leaving 'const' and 'volatile' (and, I suppose 'restrict').  The second 
strips all qualifiers, essentially yielding the TYPE_MAIN_VARIANT.

Thoughts?


r~

Comments

Marek Polacek Feb. 4, 2016, 10:09 a.m. UTC | #1
On Thu, Feb 04, 2016 at 05:01:36PM +1100, Richard Henderson wrote:
> While attempting to write some code that uses the new x86 named address
> space support in gcc 6, I found that __typeof is very unhelpful.  In
> particular, given
> 
> 	int __seg_fs *ptr;
> 	__typeof(*ptr) obj;
> 
> OBJ will not be type "int", but "int __seg_fs".  Which means that you can't
> use it to create temporaries within statement expressions.
> 
> In the process of writing this, I found a hack in __typeof added just to
> support _Atomic.  Which suggests that one of these variants would be more
> generally helpful than the hack.
> 
> I add __typeof_noas and __typeof_noqual.  The first strips only the address
> space, leaving 'const' and 'volatile' (and, I suppose 'restrict').  The
> second strips all qualifiers, essentially yielding the TYPE_MAIN_VARIANT.
> 
> Thoughts?

We've already been asked to add something like __typeof_noqual, e.g.
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39985#c3 .  In a similar PR,
I wanted to modify __typeof to drop all qualifiers, but Joseph suggested
instead to add a new typeof variant:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65455#c19
(or adjust __typeof to only drop all qualifiers on rvalues?).

So this makes sense, though I don't see the need for __typeof_noas if we
can have __typeof_noqual.

	Marek
Pedro Alves Feb. 4, 2016, 10:52 a.m. UTC | #2
On 02/04/2016 06:01 AM, Richard Henderson wrote:
> While attempting to write some code that uses the new x86 named address space 
> support in gcc 6, I found that __typeof is very unhelpful.  In particular, given
> 
> 	int __seg_fs *ptr;
> 	__typeof(*ptr) obj;
> 
> OBJ will not be type "int", but "int __seg_fs".  Which means that you can't use 
> it to create temporaries within statement expressions.
> 
> In the process of writing this, I found a hack in __typeof added just to 
> support _Atomic.  Which suggests that one of these variants would be more 
> generally helpful than the hack.
> 
> I add __typeof_noas and __typeof_noqual.  The first strips only the address 
> space, leaving 'const' and 'volatile' (and, I suppose 'restrict').  The second 
> strips all qualifiers, essentially yielding the TYPE_MAIN_VARIANT.
> 
> Thoughts?

Do we need matching __auto_type variants?
Marek Polacek Feb. 4, 2016, 11:02 a.m. UTC | #3
On Thu, Feb 04, 2016 at 10:52:28AM +0000, Pedro Alves wrote:
> On 02/04/2016 06:01 AM, Richard Henderson wrote:
> > While attempting to write some code that uses the new x86 named address space 
> > support in gcc 6, I found that __typeof is very unhelpful.  In particular, given
> > 
> > 	int __seg_fs *ptr;
> > 	__typeof(*ptr) obj;
> > 
> > OBJ will not be type "int", but "int __seg_fs".  Which means that you can't use 
> > it to create temporaries within statement expressions.
> > 
> > In the process of writing this, I found a hack in __typeof added just to 
> > support _Atomic.  Which suggests that one of these variants would be more 
> > generally helpful than the hack.
> > 
> > I add __typeof_noas and __typeof_noqual.  The first strips only the address 
> > space, leaving 'const' and 'volatile' (and, I suppose 'restrict').  The second 
> > strips all qualifiers, essentially yielding the TYPE_MAIN_VARIANT.
> > 
> > Thoughts?
> 
> Do we need matching __auto_type variants?

I think at present __auto_type removes the qualifiers from atomic types only,
but I'd hope we can just adjust __auto_type to always strip the qualifiers,
not introduce __auto_type_noqual...

	Marek
Richard Henderson Feb. 4, 2016, 4:14 p.m. UTC | #4
On 02/04/2016 10:02 PM, Marek Polacek wrote:
>> Do we need matching __auto_type variants?
>
> I think at present __auto_type removes the qualifiers from atomic types only,
> but I'd hope we can just adjust __auto_type to always strip the qualifiers,
> not introduce __auto_type_noqual...

Exactly what I was thinking.


r~
Richard Henderson Feb. 4, 2016, 4:32 p.m. UTC | #5
On 02/04/2016 09:09 PM, Marek Polacek wrote:
> We've already been asked to add something like __typeof_noqual, e.g.
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=39985#c3 .  In a similar PR,
> I wanted to modify __typeof to drop all qualifiers, but Joseph suggested
> instead to add a new typeof variant:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=65455#c19
> (or adjust __typeof to only drop all qualifiers on rvalues?).

Thanks for the pointers.  I should have expected that this would have come up 
before.  I wish I would have seen them earlier, like, say, 7 years ago...

> So this makes sense, though I don't see the need for __typeof_noas if we
> can have __typeof_noqual.

#define segment_to_flat(ptr) \
   ((__typeof_noas(*(ptr)) *) ((uintptr_t)(ptr) + segment_offset))

preserves const and volatile on the pointed-to type while stripping the address 
space.

It's something that cries out for user-defined named address spaces, but the 
latter is something that would require quite a bit more development effort to 
get right.


r~
Pedro Alves Feb. 4, 2016, 5:08 p.m. UTC | #6
On 02/04/2016 04:14 PM, Richard Henderson wrote:
> On 02/04/2016 10:02 PM, Marek Polacek wrote:
>>> Do we need matching __auto_type variants?
>>
>> I think at present __auto_type removes the qualifiers from atomic types only,
>> but I'd hope we can just adjust __auto_type to always strip the qualifiers,
>> not introduce __auto_type_noqual...
> 
> Exactly what I was thinking.

Sounds right to me.  I think it should be a design goal
for __auto_type and __typeof to be consistent.

GDB's "compile print EXPR" command (what calls gcc through the
libcc1 plugin), passes something like this to gcc:

   __auto_type __gdb_val = EXPR;

I believe we may benefit from a __auto_qual_type variant that
strips nothing, but I'm not sure.
Jeff Law Feb. 8, 2016, 4:21 p.m. UTC | #7
On 02/03/2016 11:01 PM, Richard Henderson wrote:
> While attempting to write some code that uses the new x86 named address
> space support in gcc 6, I found that __typeof is very unhelpful.  In
> particular, given
>
>      int __seg_fs *ptr;
>      __typeof(*ptr) obj;
>
> OBJ will not be type "int", but "int __seg_fs".  Which means that you
> can't use it to create temporaries within statement expressions.
ISTM __typeof is doing the obvious thing here -- what's wrong is the 
__seg_fs is inherently "associated" with the pointer, right?




>
> In the process of writing this, I found a hack in __typeof added just to
> support _Atomic.  Which suggests that one of these variants would be
> more generally helpful than the hack.
Yea, seems that way.


>
> I add __typeof_noas and __typeof_noqual.  The first strips only the
> address space, leaving 'const' and 'volatile' (and, I suppose
> 'restrict').  The second strips all qualifiers, essentially yielding the
> TYPE_MAIN_VARIANT.
Something like this seems reasonable.

jeff
Andrew MacLeod Feb. 8, 2016, 5:45 p.m. UTC | #8
mailer decided to send html I guess... for some reason.. all of a 
sudden...  stupid computer. anyway...  the original bounced., so here it 
is again...


On 02/08/2016 12:05 PM, Andrew MacLeod wrote:
> On 02/08/2016 11:21 AM, Jeff Law wrote:
>> On 02/03/2016 11:01 PM, Richard Henderson wrote:
>>
>>
>>
>>>
>>> In the process of writing this, I found a hack in __typeof added 
>>> just to
>>> support _Atomic.  Which suggests that one of these variants would be
>>> more generally helpful than the hack.
>> Yea, seems that way.
>>
>>
>
> fyi, it originated here:
>
> https://gcc.gnu.org/ml/gcc-patches/2013-08/msg00420.html
>
> snippet near the bottom:
>
>     From: "Joseph S. Myers" <joseph at codesourcery dot com>
>
>     > On Fri, 2 Aug 2013, Andrew MacLeod wrote:
>     > Ive included an additional 2 line patch which should change the
>     meaning of
>     > __typeof__  (again untested, the joys of imminently leaving for
>     2 weeks  :-).
>     > Im not sure the normal practical uses of
>     > __typeof__ have much meaning for an atomic type, it seems far
>     more useful to
>     > have __typeof__ for an atomic qualified type to return the
>     non-atomic variant.
>
>     What typeof should do in general for qualified types is unclear
>     (especially in the case of rvalues, where the movement in ISO C
>     seems to
>     be to say that rvalues can't have qualified types at all) -
>     returning the
>     non-atomic type seems reasonable to me.
>
>
> Andrew
diff mbox

Patch

diff --git a/gcc/c-family/c-common.c b/gcc/c-family/c-common.c
index 378afae..4972fe1 100644
--- a/gcc/c-family/c-common.c
+++ b/gcc/c-family/c-common.c
@@ -509,6 +509,10 @@  const struct c_common_resword c_common_reswords[] =
   { "__transaction_cancel", RID_TRANSACTION_CANCEL, 0 },
   { "__typeof",		RID_TYPEOF,	0 },
   { "__typeof__",	RID_TYPEOF,	0 },
+  { "__typeof_noas",	RID_TYPEOF_NOAS, 0 },
+  { "__typeof_noas__",	RID_TYPEOF_NOAS, 0 },
+  { "__typeof_noqual",	RID_TYPEOF_NOQUAL, 0 },
+  { "__typeof_noqual__", RID_TYPEOF_NOQUAL, 0 },
   { "__underlying_type", RID_UNDERLYING_TYPE, D_CXXONLY },
   { "__volatile",	RID_VOLATILE,	0 },
   { "__volatile__",	RID_VOLATILE,	0 },
diff --git a/gcc/c-family/c-common.h b/gcc/c-family/c-common.h
index fa3746c..6c6c2b1 100644
--- a/gcc/c-family/c-common.h
+++ b/gcc/c-family/c-common.h
@@ -100,9 +100,10 @@  enum rid
   /* C extensions */
   RID_ASM,       RID_TYPEOF,   RID_ALIGNOF,  RID_ATTRIBUTE,  RID_VA_ARG,
   RID_EXTENSION, RID_IMAGPART, RID_REALPART, RID_LABEL,      RID_CHOOSE_EXPR,
-  RID_TYPES_COMPATIBLE_P,      RID_BUILTIN_COMPLEX,	     RID_BUILTIN_SHUFFLE,
-  RID_DFLOAT32, RID_DFLOAT64, RID_DFLOAT128,
+  RID_TYPES_COMPATIBLE_P,      RID_BUILTIN_COMPLEX,
+  RID_BUILTIN_SHUFFLE, RID_DFLOAT32, RID_DFLOAT64, RID_DFLOAT128,
   RID_FRACT, RID_ACCUM, RID_AUTO_TYPE, RID_BUILTIN_CALL_WITH_STATIC_CHAIN,
+  RID_TYPEOF_NOAS, RID_TYPEOF_NOQUAL,
 
   /* C11 */
   RID_ALIGNAS, RID_GENERIC,
diff --git a/gcc/c/c-parser.c b/gcc/c/c-parser.c
index eede3a7..199a39f 100644
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -561,6 +561,8 @@  c_token_starts_typename (c_token *token)
 	case RID_STRUCT:
 	case RID_UNION:
 	case RID_TYPEOF:
+	case RID_TYPEOF_NOAS:
+	case RID_TYPEOF_NOQUAL:
 	case RID_CONST:
 	case RID_ATOMIC:
 	case RID_VOLATILE:
@@ -722,6 +724,8 @@  c_token_starts_declspecs (c_token *token)
 	case RID_STRUCT:
 	case RID_UNION:
 	case RID_TYPEOF:
+	case RID_TYPEOF_NOAS:
+	case RID_TYPEOF_NOQUAL:
 	case RID_CONST:
 	case RID_VOLATILE:
 	case RID_RESTRICT:
@@ -2530,6 +2534,8 @@  c_parser_declspecs (c_parser *parser, struct c_declspecs *specs,
 	  declspecs_add_type (loc, specs, t);
 	  break;
 	case RID_TYPEOF:
+	case RID_TYPEOF_NOAS:
+	case RID_TYPEOF_NOQUAL:
 	  /* ??? The old parser rejected typeof after other type
 	     specifiers, but is a syntax error the best way of
 	     handling this?  */
@@ -3179,7 +3185,12 @@  c_parser_typeof_specifier (c_parser *parser)
   ret.spec = error_mark_node;
   ret.expr = NULL_TREE;
   ret.expr_const_operands = true;
-  gcc_assert (c_parser_next_token_is_keyword (parser, RID_TYPEOF));
+
+  enum rid keyword = c_parser_peek_token (parser)->keyword;
+  gcc_assert (keyword == RID_TYPEOF
+	      || keyword == RID_TYPEOF_NOAS
+	      || keyword == RID_TYPEOF_NOQUAL);
+
   c_parser_consume_token (parser);
   c_inhibit_evaluation_warnings++;
   in_typeof++;
@@ -3221,9 +3232,19 @@  c_parser_typeof_specifier (c_parser *parser)
       /* 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);
+      if (ret.spec != error_mark_node
+	  && keyword == RID_TYPEOF
+	  && TYPE_ATOMIC (ret.spec))
+	keyword = RID_TYPEOF_NOQUAL;
     }
+
+  /* If requested, drop (some) qualifiers.  */
+  if (keyword != RID_TYPEOF && ret.spec != error_mark_node)
+    ret.spec = c_build_qualified_type (ret.spec,
+				       keyword == RID_TYPEOF_NOQUAL
+				       ? TYPE_UNQUALIFIED
+				       : TYPE_QUALS_NO_ADDR_SPACE (ret.spec));
+
   c_parser_skip_until_found (parser, CPP_CLOSE_PAREN, "expected %<)%>");
   return ret;
 }
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index d03b0c9..49bdf4f 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -978,6 +978,8 @@  cp_lexer_next_token_is_decl_specifier_keyword (cp_lexer *lexer)
       /* GNU extensions.  */ 
     case RID_ATTRIBUTE:
     case RID_TYPEOF:
+    case RID_TYPEOF_NOAS:
+    case RID_TYPEOF_NOQUAL:
       /* C++0x extensions.  */
     case RID_DECLTYPE:
     case RID_UNDERLYING_TYPE:
@@ -16093,19 +16095,27 @@  cp_parser_simple_type_specifier (cp_parser* parser,
       break;
 
     case RID_TYPEOF:
-      /* Consume the `typeof' token.  */
-      cp_lexer_consume_token (parser->lexer);
-      /* Parse the operand to `typeof'.  */
-      type = cp_parser_sizeof_operand (parser, RID_TYPEOF);
-      /* If it is not already a TYPE, take its type.  */
-      if (!TYPE_P (type))
-	type = finish_typeof (type);
-
-      if (decl_specs)
-	cp_parser_set_decl_spec_type (decl_specs, type,
-				      token,
-				      /*type_definition_p=*/false);
-
+    case RID_TYPEOF_NOAS:
+    case RID_TYPEOF_NOQUAL:
+      {
+	enum rid keyword = token->keyword;
+	/* Consume the `typeof' token.  */
+	cp_lexer_consume_token (parser->lexer);
+	/* Parse the operand to `typeof'.  */
+	type = cp_parser_sizeof_operand (parser, keyword);
+	/* If it is not already a TYPE, take its type.  */
+	if (!TYPE_P (type))
+	  type = finish_typeof (type);
+	/* If requested, make the type unqualified.  */
+	if (keyword != RID_TYPEOF && type != error_mark_node)
+	  type = build_qualified_type (type,
+				       keyword == RID_TYPEOF_NOQUAL
+				       ? TYPE_UNQUALIFIED
+				       : TYPE_QUALS_NO_ADDR_SPACE (type));
+	if (decl_specs)
+	  cp_parser_set_decl_spec_type (decl_specs, type, token,
+					/*type_definition_p=*/false);
+      }
       return type;
 
     case RID_UNDERLYING_TYPE:
diff --git a/gcc/testsuite/c-c++-common/typeof-noqual-1.c b/gcc/testsuite/c-c++-common/typeof-noqual-1.c
new file mode 100644
index 0000000..e4af7fb
--- /dev/null
+++ b/gcc/testsuite/c-c++-common/typeof-noqual-1.c
@@ -0,0 +1,28 @@ 
+/* { dg-do "compile" } */
+
+void foo(void)
+{
+  int i = 0;
+  const int ci = 0;
+  volatile int vi = 0;
+
+  __typeof(i) *ip = 0;
+  __typeof(ci) *cip = 0;
+  __typeof(vi) *vip = 0;
+
+  __typeof_noqual(i) *nip = 0;
+  __typeof_noqual(ci) *ncip = 0;
+  __typeof_noqual(vi) *nvip = 0;
+
+  ip = cip;		/* { dg-warning "assignment discards" } */
+  ip = vip;		/* { dg-warning "assignment discards" } */
+
+  ip = nip;
+  ip = ncip;
+  ip = nvip;
+
+  ncip = cip;		/* { dg-warning "assignment discards" } */
+  nvip = vip;		/* { dg-warning "assignment discards" } */
+
+  nip = ip;
+}
diff --git a/gcc/testsuite/gcc.target/i386/typeof-nq-seg-1.c b/gcc/testsuite/gcc.target/i386/typeof-nq-seg-1.c
new file mode 100644
index 0000000..b7decb7
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/typeof-nq-seg-1.c
@@ -0,0 +1,37 @@ 
+/* { dg-do "compile" } */
+/* { dg-options "" } */
+
+int *ip;
+int __seg_fs *fp;
+int __seg_gs *gp;
+
+void foo()
+{
+  __typeof(*fp) *fip = 0;
+  __typeof(*gp) *gip = 0;
+
+  __typeof_noqual(*fp) *qfip = 0;
+  __typeof_noqual(*gp) *qgip = 0;
+
+  __typeof_noas(*fp) *afip = 0;
+  __typeof_noas(*gp) *agip = 0;
+
+  ip = fp;	/* { dg-error "address space" } */
+  ip = gp;	/* { dg-error "address space" } */
+  ip = fip;	/* { dg-error "address space" } */
+  ip = gip;	/* { dg-error "address space" } */
+  ip = qfip;
+  ip = qgip;
+  ip = afip;
+  ip = agip;
+
+  fp = ip;	/* { dg-error "address space" } */
+  fp = gp;	/* { dg-error "address space" } */
+  fp = fip;
+  fp = qfip;	/* { dg-error "address space" } */
+  fp = afip;	/* { dg-error "address space" } */
+
+  gp = gip;
+  gp = qgip;	/* { dg-error "address space" } */
+  gp = agip;	/* { dg-error "address space" } */
+}
diff --git a/gcc/testsuite/gcc.target/i386/typeof-nq-seg-2.c b/gcc/testsuite/gcc.target/i386/typeof-nq-seg-2.c
new file mode 100644
index 0000000..74e9e3e
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/typeof-nq-seg-2.c
@@ -0,0 +1,46 @@ 
+/* { dg-do "compile" } */
+/* { dg-options "" } */
+
+int *ip;
+int __seg_fs *fp;
+int __seg_gs *gp;
+
+const int *cip;
+const int __seg_fs *cfp;
+const int __seg_gs *cgp;
+
+volatile int *vip;
+volatile int __seg_fs *vfp;
+volatile int __seg_gs *vgp;
+
+void foo()
+{
+  __typeof_noqual(*cfp) *qcfp = 0;
+  __typeof_noqual(*cgp) *qcgp = 0;
+  __typeof_noqual(*vfp) *qvfp = 0;
+  __typeof_noqual(*vgp) *qvgp = 0;
+
+  __typeof_noas(*cfp) *acfp = 0;
+  __typeof_noas(*cgp) *acgp = 0;
+  __typeof_noas(*vfp) *avfp = 0;
+  __typeof_noas(*vgp) *avgp = 0;
+
+  ip = qcfp;
+  ip = qcgp;
+  ip = qvfp;
+  ip = qvgp;
+  ip = acfp;		/* { dg-warning "assignment discards" } */
+  ip = acgp;		/* { dg-warning "assignment discards" } */
+  ip = avfp;		/* { dg-warning "assignment discards" } */
+  ip = avgp;		/* { dg-warning "assignment discards" } */
+
+  cip = acfp;
+  cip = acgp;
+  cip = avfp;		/* { dg-warning "assignment discards" } */
+  cip = avgp;		/* { dg-warning "assignment discards" } */
+
+  vip = acfp;		/* { dg-warning "assignment discards" } */
+  vip = acgp;		/* { dg-warning "assignment discards" } */
+  vip = avfp;
+  vip = avgp;
+}