diff mbox

Make C++ use the enum mode attribute

Message ID HE1PR07MB0905901340A0EB1306352696E4D40@HE1PR07MB0905.eurprd07.prod.outlook.com
State New
Headers show

Commit Message

Bernd Edlinger Feb. 7, 2016, 11:24 a.m. UTC 
On 06.02.2016 at 23:37 Trevor Saunders wrote:
> On Sat, Feb 06, 2016 at 08:36:27PM +0000, Bernd Edlinger wrote:
>>
>> Hi Jakub,
>>
>> as a follow-up for your c/69669 fix, I'd like to have the enum mode also honored
>> in C++ code, because the mode attribute now finally really works in C, but it is
>> completely and silently(!) ignored by C++ code, which results in incompatible
>> code.
>>
>> So I duplicated what is done in c/c-decl.c also in cp/decl.c.  That worked
>> immediately, except that it is not possible to explicitly check the "mode"
>> attribute in the TYPE_ATTRIBUTES (enumtype) because that attribute
>> is never copied to the type, and the original attribute list is not available
>> here.  That should be OK, as this check was added to fix pr52085 which
>> does not apply here, because C++ does not support enum forward
>> declarations.
>
> it is allowed if you do something like
>
> enum X : int;
>

Oh I see, that is new in C++11.
I was referring to a GCC extension of the C syntax:

enum X;
/* enum X can be used here to declare pointers for instance */
enum X
{
  A=1,
  B=2
};

That syntax is not understood by C++, while the enum with an
underlying type is not understood by C code.  The mode attribute
is understood by both.

BTW I tried:

enum __attribute__((packed)) X : short;
enum X : short
{
  x1 = 1,
  x2 = 2
};

here the packed attribute is ignored, again without a warning.
There was a warning for that in gcc4.8 at least.

I think the warning is not generated because the decl_attributes does
not see the underlying type.  Thus, does the following look reasonable?

        if (CP_INTEGRAL_TYPE_P (underlying_type))
@@ -13196,6 +13194,8 @@ start_enum (tree name, tree enumtype, tree underly
                 underlying_type, enumtype);
      }

+  cplus_decl_attributes (&enumtype, attributes, 
(int)ATTR_FLAG_TYPE_IN_PLACE);
+
    /* If into a template class, the returned enum is always the first
       declaration (opaque or not) seen. This way all the references to
       this type will be to the same declaration. The following ones are 
used


> but it seems really pointless to support setting the size of something
> when the language gives you a standard way to do that.
>

This is about interoperability with C code.
And that is really essential IMO.

Thanks
Bernd.

>> If that patch is not appropriate for stage  4, I would at least want to emit
>> an attribute directive ignored warning in c++ mode.  I think that could
>> be done in handle_mode_attribute.  But fixing that feature is cooler.
>
>   Its definitely bad to sighlently ignore the attribute.  On one hand
>   being compatable with C seems nice, but on the other hand in C++ 11 and
>   later you can set the size without using attributes, and the mode thing
>   has always seemed a bit odd to me since its not really clear what C
>   type they corespond to.
>
>   Trev
>
>>
>> Boot-strapped and regression tested on x86_64-pc-linux-gnu.
>> Ok for trunk?
>>
>>
>> Thanks
>> Bernd.
>
>> cp:
>> 2016-02-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* decl.c (finish_enum_value_list): Use the specified mode.
>>
>> testsuite:
>> 2016-02-06  Bernd Edlinger  <bernd.edlinger@hotmail.de>
>>
>> 	* c-c++-common/pr69669.c: Check the used mode.
>
>> Index: gcc/cp/decl.c
>> ===================================================================
>> --- gcc/cp/decl.c	(Revision 233176)
>> +++ gcc/cp/decl.c	(Arbeitskopie)
>> @@ -13310,6 +13310,19 @@ finish_enum_value_list (tree enumtype)
>>         use_short_enum = flag_short_enums
>>           || lookup_attribute ("packed", TYPE_ATTRIBUTES (enumtype));
>>
>> +      /* If the precision of the type was specified with an attribute and it
>> +	 was too small, give an error.  Otherwise, use it.  */
>> +      if (TYPE_PRECISION (enumtype))
>> +	{
>> +	  if (precision > TYPE_PRECISION (enumtype))
>> +	    error ("specified mode too small for enumeral values");
>> +	  else
>> +	    {
>> +	      use_short_enum = true;
>> +	      precision = TYPE_PRECISION (enumtype);
>> +	    }
>> +	}
>> +
>>         for (itk = (use_short_enum ? itk_char : itk_int);
>>              itk != itk_none;
>>              itk++)
>> Index: gcc/testsuite/c-c++-common/pr69669.c
>> ===================================================================
>> --- gcc/testsuite/c-c++-common/pr69669.c	(Revision 233176)
>> +++ gcc/testsuite/c-c++-common/pr69669.c	(Arbeitskopie)
>> @@ -1,5 +1,6 @@
>>   /* PR c/69669 */
>>   /* { dg-do compile } */
>> +/* { dg-options "-fdump-rtl-final" } */
>>
>>   enum __attribute__((mode(QI))) E { F = 1 };
>>
>> @@ -8,3 +9,5 @@ foo (enum E *x, int y)
>>   {
>>     *x = (enum E) y;
>>   }
>> +
>> +/* { dg-final { scan-rtl-dump-times "mem:QI" 1 "final" } } */
>
diff mbox

Patch

Index: decl.c
===================================================================
--- decl.c	(revision 233200)
+++ decl.c	(working copy)
@@ -13180,8 +13180,6 @@  start_enum (tree name, tree enumtype, tree underly

    SET_SCOPED_ENUM_P (enumtype, scoped_enum_p);

-  cplus_decl_attributes (&enumtype, attributes, 
(int)ATTR_FLAG_TYPE_IN_PLACE);
-
    if (underlying_type)
      {