Message ID | HE1PR07MB0905CBC2F8154F44BB32E14AE4D30@HE1PR07MB0905.eurprd07.prod.outlook.com |
---|---|
State | New |
Headers | show |
On Feb 6, 2016, at 12:36 PM, Bernd Edlinger <bernd.edlinger@hotmail.de> wrote:
> If that patch is not appropriate for stage 4
Without looking at the patch, fixes to abi incompatibilities I think are important. Would be nice to fix them.
[ ok, I peeked at the patch ]
Wow, nice, short and sweet. I’d like for it to be considered.
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; but it seems really pointless to support setting the size of something when the language gives you a standard way to do that. > 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" } } */
On Feb 6, 2016, at 2:37 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote: > it is allowed if you do something like > > enum X : int; > > but it seems really pointless to support setting the size of something > when the language gives you a standard way to do that. Yes, it is, as long as you never use header files, and multiple languages. :-) Unfortunately, those are vended features of C and C++. Once you have a C project that you want to be able to be used from C++, it then is handy to be able to do this.
On Mon, Feb 08, 2016 at 01:41:03PM -0800, Mike Stump wrote: > On Feb 6, 2016, at 2:37 PM, Trevor Saunders <tbsaunde@tbsaunde.org> wrote: > > it is allowed if you do something like > > > > enum X : int; > > > > but it seems really pointless to support setting the size of something > > when the language gives you a standard way to do that. > > Yes, it is, as long as you never use header files, and multiple languages. :-) Unfortunately, those are vended features of C and C++. Once you have a C project that you want to be able to be used from C++, it then is handy to be able to do this. Sure, but how many people use the mode attribute on enums? Probably more than one, and I guess it doesn't really hurt that much so whatever. and yes I admitt I'm a little biased against this attribute in the first place ;) Trev >
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" } } */