tweeny icon indicating copy to clipboard operation
tweeny copied to clipboard

Warning "conversion from ‘int’ to ‘float’ may change value" when using integer values

Open klapstoelpiloot opened this issue 4 years ago • 4 comments

When using integer values such as in this example:

auto t = tweeny::from(130).to(130).during(300).to(80).during(400);

The compiler issues warnings for the conversion from int to float for all the different easings, sometimes multiple warnings per easing. I don't want to disable the warnings, because they may be valuable information pointing out where a possible bug exists.

It would be nice if a static_cast<float>() is used where appropriate, so that my warnings list is not flooded.

klapstoelpiloot avatar Jan 01 '22 19:01 klapstoelpiloot

Can you tell me which compiler you're using and compiler flags?

mobius3 avatar Jan 01 '22 19:01 mobius3

GCC (Raspbian 8.3.0-6+rpi1) 8.3.0 I have a lot of flags, but -W"conversion" and -Wall are probably what enables these warnings. I have to say that these are legitimate warnings, as the conversion could be unintended and/or causing a bug.

I have temporarely solved the problem like this:

#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Wconversion"
#include "tweeny/tweeny.h"
#pragma GCC diagnostic pop

But this is, of course, not a proper fix.

klapstoelpiloot avatar Jan 01 '22 20:01 klapstoelpiloot

I have to say that these are legitimate warnings,

Not saying they aren't! I take warnings seriously, it's just that the last time I compiled something with tweeny myself I had no warnings, mostly because -Wconversion isn't enabled with -Wall? or wasn't, at least.

I wanted to check which variables are giving out these warnings as, giving the template-heavy nature of tweeny, simply casting it with static cast might break things with user-defined classes and user-defined casting.

I'll reproduce it and fix as soon as possible, thanks for your workaround for GCC, though.

mobius3 avatar Jan 01 '22 20:01 mobius3

Right, so, it is as I thought. I cannot simply static_cast<float> all the easing values because it will break composite types.

Consider:

struct boxed_float {
    float value;
    boxed_float() : value(0) { }
    boxed_float(float v) : value(v) { }
};

boxed_float operator+(const boxed_float & a, const boxed_float & b) { return {a.value + b.value}; }
boxed_float operator-(const boxed_float & a, const boxed_float & b) { return {a.value - b.value}; }
boxed_float operator-(const boxed_float & a) { return {-a.value}; }
boxed_float operator*(const boxed_float & a, const boxed_float & b) { return {a.value * b.value}; }
boxed_float operator/(const boxed_float & a, const boxed_float & b) { return {a.value / b.value}; }

...

auto t = tweeny::from(boxed_float{1.0f}).to(boxed_float{100.0f}).during(300).via(tweeny::easing::circularIn);

Supporting composite types is a feature I value in tweeny so we need another solution here.

  • Use std::enable_if to cast if it's an integral type, and not cast if its not. This is what's done for example in the linear easing and would be a proper solution but would lead to massive amounts of code duplication which makes me lean away from this solution.
  • Try to pump tweeny::interpolate call to detect if the value is an integral/can be casted to float and then do it before calling the easing function
    • this might break user-defined easing functions (e.g, an easing that is for a specific T that can be constructed with an integer but not floats)
  • Turn off -Wconversion just for this file. Not a proper fix but there is no single casting solution to this problem so we might as well "roll" with it.

I'm truly tempted to go with the last one, at least for now. Any more input/opinions on this?

mobius3 avatar Jan 31 '22 00:01 mobius3