public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: "Sam van Kampen via gcc-patches" <gcc-patches@gcc.gnu.org>
To: Martin Sebor <msebor@gmail.com>
Cc: gcc-patches@gcc.gnu.org
Subject: Re: [patch, c++] Add a warning flag for the enum bit-field declaration warning in bug #61414.
Date: Wed, 18 Oct 2017 19:29:00 -0000	[thread overview]
Message-ID: <20171018191516.h5ewt3z6wsvgauvs@segfault.party> (raw)
In-Reply-To: <f0f3ece2-b213-ff33-9b26-d283fcbf90c9@gmail.com>

On Wed, Oct 18, 2017 at 09:46:08AM -0600, Martin Sebor wrote:
> > Fair enough, I didn't know whether to change the way it currently was
> > triggered. Do you think it should fall under -Wextra (I don't think it
> > falls under -Wall, since it isn't "easy to avoid or modify to prevent
> > the warning" because it may be valid and wanted behavior), or should it
> > be enabled by no other flag?
> 
> I think it depends on the implementation of the warning.  With
> the current (fairly restrictive) behavior I'd say it should be
> disabled by default.  But if it were to be changed to more closely
> match the Clang behavior and only warn for bit-field declarations
> that cannot represent all enumerators of the enumerated type, then
> including it in -Wall would seem helpful to me.
> 
> I.e., Clang doesn't warn on this and IIUC that's what the reporter
> of the bug also expects:
> 
>   enum E: unsigned { E3 = 15 };
> 
>   struct S { E i: 4; };
> 
> (There is value in warning on this as well, but I think most users
> will not be interested in it, so making the warning a two-level one
> where level 1 warns same as Clang and level 2 same as GCC does now
> might give us the best of both worlds).

I see what you mean - that is the behavior I wanted to implement in the
first place, but Jonathan Wakely rightly pointed out that when an
enumeration is scoped, all values of its underlying type are valid
enumeration values, and so the bit-field you declare in 'S' _is_ too
small to hold all values of 'enum E'.

Here's the corresponding text from draft N4659 of the C++17 standard,
§10.2/8 [dcl.enum]

For an enumeration whose underlying type is fixed, the values of the
enumeration are the values of the underlying type. [...] It is possible
to define an enumeration that has values not defined by any of its
enumerators.

Still, warning when a bit-field can't hold all enumerators instead of
all values may be a good idea. I've looked into it, and it does require
recalculating the maximum and minimum enumerator value, since the bounds
of the underlying type are saved in TYPE_MIN_VALUE and TYPE_MAX_VALUE
when the enumeration is scoped, instead of the min/max enumerator value.

Would adding that separate warning level be part of a separate patch, or
should I add it to this one?

  reply	other threads:[~2017-10-18 19:11 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-10-16 11:20 Sam van Kampen via gcc-patches
2017-10-16 13:26 ` Sam van Kampen via gcc-patches
2017-10-17  0:17   ` Joseph Myers
2017-10-18 13:59     ` Sam van Kampen via gcc-patches
2017-10-17  7:14   ` Martin Sebor
2017-10-18 13:54     ` Sam van Kampen via gcc-patches
2017-10-18 16:00       ` Martin Sebor
2017-10-18 19:29         ` Sam van Kampen via gcc-patches [this message]
2017-10-19  3:23           ` Martin Sebor
2017-10-20 16:05           ` Jason Merrill
2019-11-25 14:40             ` Jakub Jelinek
2019-11-25 14:54               ` Jakub Jelinek
2019-11-25 23:42                 ` [C++ PATCH] Fix up is too small to hold all values of enum warning (PR c++/61414) Jakub Jelinek
2019-11-26 21:52                   ` Jason Merrill
2017-10-19 19:22     ` [patch, c++] Add a warning flag for the enum bit-field declaration warning in bug #61414 Eric Gallager
2017-10-19 23:07       ` Martin Sebor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20171018191516.h5ewt3z6wsvgauvs@segfault.party \
    --to=gcc-patches@gcc.gnu.org \
    --cc=msebor@gmail.com \
    --cc=sam@segfault.party \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).