Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
klapstoelpiloot opened this issue Jan 1, 2022 · 4 comments

Comments

@klapstoelpiloot
Copy link

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.

@mobius3
Copy link
Owner

mobius3 commented Jan 1, 2022

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

@klapstoelpiloot
Copy link
Author

klapstoelpiloot commented Jan 1, 2022

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.

@mobius3
Copy link
Owner

mobius3 commented Jan 1, 2022

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
Copy link
Owner

mobius3 commented Jan 31, 2022

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants