Really -Wconversion is correct to warn about bit-fields because the conversion will lose bits.

Subject: Re:  -Wconversion useless

You miss the point. The only way to assign a non-constant value to a bit
field outside of a struct is using an integral variable i.e.,

struct foo
{
    int a : 2;
};

void assign( struct foo v, int x )
{
    v.a = x;
}

This results automatically in a warning. How do code this assignment
type-safe? There is no (bit-field) cast operator in the C or C++.

Like the "gcc" code base, our code does a lot bit-field assignments. We
no get thousands of warning using -Wconversion, this behavior makes the
option useless.

Again, compile the "gcc" code-base with "-Wconversion" and then you will
understand the problem.

Expect for bit-fields which are problematic to do potential bit-loss
(you must use with caution), you want warnings for implicit narrowing if
values, e.g., (double -> int)

void foo( int x );

...

double n;
foo(n)

The old behavior was just fine!

I'm sure many people do not even realize they are NOT getting implicit
conversions warnings anymore because they are not caught by "-Wall"
anymore. We only discovered this be tracing a bug in our code! We then
turned on "-Wconversion" only to discover thousands of warnings with no
way practical way to fix them. 

Regards,

Tom Geocaris

On Thu, 2009-02-12 at 17:39 +0000, pinskia at gcc dot gnu dot org wrote:
> 
> ------- Comment #1 from pinskia at gcc dot gnu dot org  2009-02-12 17:39 -------
> Really -Wconversion is correct to warn about bit-fields because the conversion
> will lose bits.
> 
> 

> The old behavior was just fine!

You absolutely did not understand what the old -Wconversion did. 

http://gcc.gnu.org/wiki/NewWconversion

But if you still want the old behaviour, just use -Wtraditional-conversion.

> I'm sure many people do not even realize they are NOT getting implicit
> conversions warnings anymore because they are not caught by "-Wall"

See the FAQ in the link above.

> anymore. We only discovered this be tracing a bug in our code! We then

The new Wconversion is mentioned in the changes of GCC 4.3.

> turned on "-Wconversion" only to discover thousands of warnings with no
> way practical way to fix them. 

I admit it is unfortunate that there is no way to specify casts to bit-fields. But that is hardly the fault of Wconversion. Nonetheless, it could be possible to use bitwise-and to tell Wconversion that some conversion is really desired. Example:

struct foo
{
    int a : 2;
};

void assign( struct foo v, int x )
{
    v.a = x & 0x3;
}

Do you think this would be an acceptable solution? (I don't know if this works now in GCC 4.4)

Manuel,

You miss understood what I meant by "old behavior was just fine". I was saying that the previous behavior of "gcc" worked fine and I was NOT referring specifically to the "-Wconversion" option. 

The previous version of "gcc" warned when implicit narrowing of doubles to integral values, such as

    double n = 0.0000000005;
    int d = n;

when using the "-Wall" option. 

The behavior of "gcc" has changed. Moving all the conversion warnings to fall under "-Wconversion" may make semantic sense, but it alters the behavior of "gcc". 

We can fault ourselves for missing this change in the documentation, but there a level of expectation that the fundamental behavior of the compiler is consistent from release-to-release. And when fundamental behavior of "gcc" changes, ample notice should be given. People need to change Makefiles, alter code (if possible), etc... 

With regard to "-Wtraditional-conversion", it does not work when compiling "C++" code.

> Do you think this would be an acceptable solution? (I don't know if this works
> now in GCC 4.4)

Absolutely not. It's not a portable solution. There is nothing in the "C++" standard (that I'm aware of) that suggests that "anding" an integral value with a "constant" value results in a truncated integral value. It's a bad hack.

As you say, its is unfortunate that "C" and "C++" do not have a bit-field "cast-operators". But that is the reality. 

There is a lot of code written using "bit-fields". Look at "gcc" itself. Until the "C" or "C++" language contains a "type-safe" construct, such as a cast-operator, "gcc" should not issue a warning by default.

If you have ideas on how to solve this, please submit them to the "C" or "C++" standards group.

The "gcc" 4.3 stream is not safe (for us) to use as is. We need the compiler to issue warnings when implicit narrowing occurs (expect in the case of a bit-field). As is, we get thousands of warnings from our code when using "-Wconversion". Consequently, we are forced disable "-Wconversion" to suppress the bit-field warnings at the risk of missing other narrowing warnings. And this is not acceptable to us.

We've already moved back to the 4.2 "gcc".

Best Regards,

Tom Geocaris

(In reply to comment #4)
> 
> The previous version of "gcc" warned when implicit narrowing of doubles to
> integral values, such as
> 
>     double n = 0.0000000005;
>     int d = n;
> 
> when using the "-Wall" option.

AFAIK, that is not true. I just tried your very example with gcc 4.2.4 and it doesn't warn with -Wall -Wextra -Wconversion. g++ did warn but not with -Wall, you still needed to specify -Wconversion, and it did not warn in many cases. So please, check your facts.

> The behavior of "gcc" has changed. Moving all the conversion warnings to fall
> under "-Wconversion" may make semantic sense, but it alters the behavior of
> "gcc". 

Fixing bugs alters behaviour. The change of behaviour was documented beyond what is normally expected.

> We can fault ourselves for missing this change in the documentation, but there
> a level of expectation that the fundamental behavior of the compiler is
> consistent from release-to-release. And when fundamental behavior of "gcc"
> changes, ample notice should be given. People need to change Makefiles, alter
> code (if possible), etc... 

The behaviour of the compiler changes *every* release in many aspects. That is exactly what http://gcc.gnu.org/gcc-X.X/changes.html is for.

> With regard to "-Wtraditional-conversion", it does not work when compiling
> "C++" code.

Of course it doesn't. Have you understood what -Wtraditional-converion (and the old -Wconversion) actually warned for?

> Absolutely not. It's not a portable solution. There is nothing in the "C++"
> standard (that I'm aware of) that suggests that "anding" an integral value with
> a "constant" value results in a truncated integral value. It's a bad hack.
> 

Really? Then I have no ideas. In any case, someone else would need to take care of this because I do not have time. http://gcc.gnu.org/faq.html#support

> If you have ideas on how to solve this, please submit them to the "C" or "C++"
> standards group.

bit-field cast operators?

> We've already moved back to the 4.2 "gcc".

Good that you found a solution. I will leave this open in case someone finds a better solution and decides to submit a patch.

Subject: Re:  cannot silence -Wconversion warnings for
	bit-fields


> AFAIK, that is not true. I just tried your very example with gcc 4.2.4 and it
> doesn't warn with -Wall -Wextra -Wconversion. g++ did warn but not with -Wall,
> you still needed to specify -Wconversion, and it did not warn in many cases. So
> please, check your facts.

You are correct in 4.2 you still need the -Wconversion option. We
upgraded from 3.4.6 to 4.2/4.3. In 3.4.6, gcc warned of this error
without any additional options.

> Fixing bugs alters behaviour. The change of behaviour was documented 
> beyond what is normally expected.

You are splitting hairs. I don't see this change as a bug-fix. It's
along the lines to reinterpreting the "C" or "C++" language with regard
to bit-field assignments. The only way "C" or "C++" to assign a integral
variable a bit field is an assignment:

   struct A
   {
      unsigned int v : 2;
   }

   void foo( A * a, int v ) { a->v = v; }

For which "gcc" now issues an warning for, always! And there is no
"language" defined way to eliminate this warning. Outside of writing
something ugly like:

 struct A
 {
    union {
       unsigned int v : 2;
       unsigned int fill;
    };
  };

  void foo( A * a, int v )
  {
    a->fill |= v & 0x3;
  }

And if I have to write this, I might as well not use a bit-field!

Again, gcc 4.3.x now issues thousands of warnings (in our code) for
which we have NO reasonable and portable way to clean the code and NO
way to suppress the warning.

It makes the compiler (for us) not usable.

> Of course it doesn't. Have you understood what -Wtraditional-converion (and the
> old -Wconversion) actually warned for?

I don't care what "-Wconversion" previously did. We never used it, we
did not need to! In 3.4.6 the compiler issued the implicit conversion
warnings without additional options. In 4.2/4.3 you need -Wconversion to
get these warnings. In 4.2.4, "gcc" doesn't warn about bit-fields, but
in 4.3.x it does.

> Really? Then I have no ideas. In any case, someone else would need to 
> take care of this because I do not have time. >
> http://gcc.gnu.org/faq.html#support

I don't understand why this change was made when "C" and the "C++"
language has no support for it... 

Given this has not been been an issue with "C" for over 30 years, there
is probably a reason it is not in the language.

I agree with Tom, the new behavior of -Wconversion is useless with bitfields, this should be fixed so that we can use -Wconversion again.
Why is this bug on "WAITING"?

With -Wconversion for the assignment to bitfields gcc 4.4.2 gives a
warning, which is impossible to fix.

This BUG (I hope everybody agrees it is a BUG) gives us a lot of
trouble while porting our code (45 developers, 7 year history) from
4.2.4 to 4.4.2

We spent a lot of effort to make our code clean with 
-Wshadow -Wconversion -Wall -Werror flags and now our only choice is
to remove -Wconversion.

Please assign and fix this ASAP.

Thank you, Zach.

I was wrong, the warning is correct and there is a way to fix it.

***1. The easy recipe: For the assignment to bit-fileds use unsigned int bit-field on the left and mask the right side with the appropriate mask.

***2. Explanation given by Alexander Smundak from Cisco:
Actually, the compiler is doing the right thing. Let's simplify it:
--------------
struct data_t {
        int bit:1;
} data;

void foo() {
        data.bit = 1;
}
-----------------
$ gcc.c4.4.0-p0 -fsyntax-only -Wall -Wconversion wbit.cpp
wbit.cpp: In function ‘void foo()’:
wbit.cpp:6: warning: conversion to ‘signed char:1’ alters ‘int’ constant value

`int' is signed, meaning that the highest bit of data.bit is to be extended when moved to a longer integer. Which means that the values for the 1-bit sized integer are 0 and -1. Indeed, if I change the assignment above to 
        data.bit = -1;
the code will compile without warnings. And indeed that's what happens.
Likewise, for the 2-bit bit fields the possible values are
0,1,-1 and -2. Etc.

***3. Here is a little code example to try with g++ -Wconversion b.cpp

-----------------------------------------------------------------
// b.cpp
#define M23 ((1 << 23) - 1)
#define M24 ((1 << 24) - 1)


class xxx {
public:
  xxx():u_24(0), i_24(0) {}
  unsigned int u_24:24;
  int i_24:24;
};

int main(int argc, char** argv) {
  xxx x;
  unsigned int y = 0xffff;
  x.u_24 = M24 & y;
  x.i_24 = M23 & y;
  x.i_24 = M24 & y; // warning: conversion to  int:24  from  int  may alter its value
}
---------------------------------------------------------

Thanks, Zach.

Hi Manuel,

I think it is a good idea to warn about narrowing both from a type to another type, and from a type to a bit-field.  For new codes, one should use the bit-masking technique for bit-field narrowing just as one should use type-casting for general narrowing.

However, with so many lines of legacy code out there using bit-filed that have been proven to work, it doesn't make sense to revisit/modify them.  Would it be possible for gcc to provide a -Wbitfield-conversion flag in new releases and make 39170 an enhancement (preferably high priority)?

(In reply to comment #10)
> However, with so many lines of legacy code out there using bit-filed that have
> been proven to work, it doesn't make sense to revisit/modify them.  Would it be
> possible for gcc to provide a -Wbitfield-conversion flag in new releases and
> make 39170 an enhancement (preferably high priority)?

Seriously, I am not the "maintainer" of GCC diagnostics or even Wconversion. Anyone can contribute!

Yes, it may be possible to provide such an option. I am not against such an option and even if I were I cannot stop you from implementing it and getting it approved by GCC maintainers. I don't know how much work it would be to implement it, and out of kindness I might invest some of my free time to try to do it for GCC 4.6. But even if I might try, that won't happen before summer almost for sure, and then GCC 4.6 won't be available before late next year.

Is there any resolution to this issue? We need to move to a more recent version of gcc, but are still stuck at gcc 4.2.4. 

I looked at gcc 4.7.2 but behavior is the same and I don't see an option to suppress these warnings.

(In reply to comment #12)
> Is there any resolution to this issue? We need to move to a more recent version
> of gcc, but are still stuck at gcc 4.2.4. 

I think the best option would be to have a GCC extension for casting bit-field types (int:2) so they can be used to silence false positives. But this may be rejected by the C/C++ FE maintainers, so unless you get their approval, I wouldn't try to pursue it.

The second best option, and probably easier to implement, is to have a new option -Wconversion-bitfields that gives the Wconversion warning for bitfields and use that to control the warning code like:

      if (unsafe_conversion_p (type, expr, true))
	warning_at (loc, TYPE_IS_BITFIELD(type) ? OPT_Wconversion_bitfield : OPT_Wconversion,
		    "conversion to %qT alters %qT constant value",
		    type, expr_type);
      return;

I don't even know if TYPE_IS_BITFIELD exists or there is an equivalent API.

Personally, I have no free time to work on this, so someone else will have to do it. I can give some hints on what you would need to do to implement it if someone is interested and needs guidance.

(In reply to Jeng-Liang Tsai from comment #10)
> Would it be possible for gcc to provide a -Wbitfield-conversion flag in new
> releases and make 39170 an enhancement (preferably high priority)?

Just an FYI, there's a proposal to add a -Wbitfield-conversion flag that does something slightly different than the behavior described in this issue; for reference, see: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00723.html
I'd be fine with it doing either the behavior in that patch, or the behavior from this issue...

(In reply to Eric Gallager from comment #14)
> Just an FYI, there's a proposal to add a -Wbitfield-conversion flag that
> does something slightly different than the behavior described in this issue;
> for reference, see: https://gcc.gnu.org/ml/gcc-patches/2014-11/msg00723.html
> I'd be fine with it doing either the behavior in that patch, or the behavior
> from this issue...

I think the two behaviors are orthogonal. People still need an option that refines -Wconversion and allows silencing it for bit-fields. I will prefer if this option is called -Wbitfield-conversion and the other proposal is called something else (-Wbitfield-promotion ? -Wbitfield-shift ?).

But whoever does the implementation work, will decide the name.

Here is a type safe C++ solution:

template<unsigned int N, typename T> union NBitValueUnion
{
        static_assert((sizeof(T)*8) > N, "Strange Bitfield Width");
private:
        const T Data;
public:
        struct
        {
                const T Value : N;
                const T : (sizeof(T)*8-N);
        };
        NBitValueUnion(const T& _Data): Data(_Data) {}
};
template<unsigned int N, typename T> const NBitValueUnion<N, T> NBitValue(const T& _Data)
{ return NBitValueUnion<N, T>(_Data); }
 

Usage:
Bitfield.Member = VariableName; //Triggers -Wconversion
Bitfield.Member = NBitValue<FieldWidth>(VariableName).Value; //Silent.

Unfortnunately it triggers -Waggregate-return (which is a topic for it self since its a type thats fits in a register, so its an aggregate.. but not in hardware.)

Any improvements are welcome.

Warning if the code 'may alter its value' is one thing.

Can we have GCC at least not warn if it knows it will not alter the value?

For example, compiling

struct foo
{
   unsigned bar: 30;
   unsigned fill: 2;
};

struct foo test(unsigned value)
{
   struct foo foo;

   foo.bar = value >> 2;

   return foo;
}

This yields:

test.c: In function ‘test’:
test.c:11:14: warning: conversion to ‘unsigned int:30’ from ‘unsigned int’ may alter its value [-Wconversion]
    foo.bar = value >> 2;
              ^

Tested with GCC 5.4.0 and 7.2.

Even this version creates a warning:

#include <stdint.h>

struct foo
{
   unsigned bar: 30;
   unsigned fill: 2;
};

struct foo test(uint32_t value)
{
   struct foo foo;

   foo.bar = (value >> 2) & 0x3fffffffU;

   return foo;
}

(In reply to Manuel López-Ibáñez from comment #11)
> (In reply to comment #10)
> > However, with so many lines of legacy code out there using bit-filed that have
> > been proven to work, it doesn't make sense to revisit/modify them.  Would it
> > be possible for gcc to provide a -Wbitfield-conversion flag in new releases
> > and make 39170 an enhancement (preferably high priority)?
> 
> Seriously, I am not the "maintainer" of GCC diagnostics

That's David Malcolm; adding him on cc

(In reply to Eric Gallager from comment #19)
> (In reply to Manuel López-Ibáñez from comment #11)
> > (In reply to comment #10)
> > > However, with so many lines of legacy code out there using bit-filed that have
> > > been proven to work, it doesn't make sense to revisit/modify them.  Would it
> > > be possible for gcc to provide a -Wbitfield-conversion flag in new releases
> > > and make 39170 an enhancement (preferably high priority)?
> > 
> > Seriously, I am not the "maintainer" of GCC diagnostics
> 
> That's David Malcolm; adding him on cc

...and since he's now made himself the assignee, changing status to ASSIGNED

Possibly related to bug 87406 which requests adding clang's -Wbitfield-constant-conversion

*** Bug 95213 has been marked as a duplicate of this bug. ***