[patch, c++] Add a warning flag for the enum bit-field declaration warning in bug #61414.

Jakub Jelinek jakub@redhat.com
Mon Nov 25 14:40:00 GMT 2019


On Fri, Oct 20, 2017 at 11:59:39AM -0400, Jason Merrill wrote:
> > 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?
> 
> I think making that behavior the default would be appropriate.  The
> current behavior for scoped enums seems like a bug; sure, all values
> of the underlying type are valid, but that's also true for "unsigned i
> : 4", and we don't warn about that.

Note, the chosen switch name -Wbitfield-enum-conversion looks wrong to me,
that is a (non-default) warning option that clang implements, but it
actually is completely different warning, which GCC doesn't implement and
the warning GCC has is something clang on the other side doesn't implement.
So reusing the same option name for something different is bad.
The warning clang++ implements is warning when storing a value with enum
type into a bitfield with implicit cast, if the number of bitfield bits (the
bitfield could very well be just int x : 2; or unsigned y : 3; etc.) is
smaller than what is needed for the range of the enum.  So, if we want to
add a warning option for this warning, it should be something like
-Wbitfield-enum-width or so.

Agreed on that we just should only consider the range of enum values and not
the underlying type for the warning, whether the warning has a warning
switch or not.

I think the reason why for C++ the warning is implemented by checking the
precision of the underlying type is that this way was the cheapest, at least
when scoped enums or enums with fixed underlying type or mode attribute
isn't used, then finish_enum_value_list already computed the needed
precision.  While e.g. in the C FE the same warning is implemented by
remembering the minimum and maximum values in type_lang_specific (although,
that seems like a waste, why it doesn't just store the minimum precision
instead of the two trees).
finish_enum_value_list already computes almost everything even when using
fixed underlying type or mode attribute, except the
      signop sgn = tree_int_cst_sgn (minnode) >= 0 ? UNSIGNED : SIGNED;
      int lowprec = tree_int_cst_min_precision (minnode, sgn);
      int highprec = tree_int_cst_min_precision (maxnode, sgn);
      int precision = MAX (lowprec, highprec);

I guess the question is, shall we store the minimum precision needed
somewhere by finish_enum_value_list (perhaps only bother if the precision
of the underlying type is not the same) or compute it each time again
(which would mean for each bitfield of enum type walk the list of all the
enum values)?  And, if we should store it somewhere, any preferences where?
These days C++ FE has TYPE_LANG_SPECIFIC just for classes, so options are
just use the class TYPE_LANG_SPECIFIC and just stick the min/max values in
some trees in there, restore having different lang specific variants and
store just minimum precision in there, add hash map from tree (hashed using
TYPE_UID) to precision, something else?

	Jakub



More information about the Gcc-patches mailing list