sonar-dotnet icon indicating copy to clipboard operation
sonar-dotnet copied to clipboard

Fix S1905 FP: Floating point casts are not redundant

Open doterik opened this issue 1 year ago • 6 comments

Description

I stumbled upon this somewhat dated blog post (by @jaredpar) https://blog.paranoidcoding.org/2014/12/22/redundant-cast.html.

This made me question whether S1905 correctly handles casting to float/double.

Repro steps

float x = GetValue();
float y = z / (float)(x * 2);

Expected behavior

It might be a FP, according to the article "Floating point casts are rarely redundant". Although there may have been changes since it was written, it could still be worth considering.

Actual behavior

S1905 Remove this unnecessary cast to 'float'.

Related information
* Visual Studio 17.11.0
* .NET 9
* SonarAnalyzer 9.28.0.94264

doterik avatar Jul 03 '24 14:07 doterik

The post is a bit old but the behavior it describes still exists. It's very challenging to identity truly redundant floating point casts. A safe, but very conservative approach, is only raising the warning when the RHS is a local, array element or field.

jaredpar avatar Jul 03 '24 14:07 jaredpar

@jaredpar Thank you for your comment.

My main concern was about heeding the warning, removing the cast, and then not grasping the potential consequences, especially in unfamiliar code.

It seems this might only be relevant in rare and specific instances, so the warning could possibly stay, or, in the case of floating-point casts, be modified to a suggestion.

doterik avatar Jul 03 '24 17:07 doterik

Hey @doterik , thanks for raising this. After reading Jared's post, I can confirm this is an FP. I added a reproducer and we will tackle this at some hardening effort in the near future.

Thanks a lot to both of you!

@gregory-paidis-sonarsource 👍🏼

SharpLab.io seems to agree...

https://gist.github.com/doterik/b529113a92e37e306805d1c2359bc7ee

image

doterik avatar Jul 11 '24 21:07 doterik

@jaredpar Fascinating post! Out of curiosity, do you have an example with actual numbers where removing the cast changes the result?

Tim-Pohlmann avatar Jul 12 '24 08:07 Tim-Pohlmann

Consider this code

class Program
{
    static void Main()
    {
        Console.WriteLine(M1(float.Epsilon));
        Console.WriteLine(M2(float.Epsilon));
    }

    static bool M1(float f) => (f / 2.0f) == 0;
    static bool M2(float f) => (float)(f / 2.0f) == 0;
}

On .NET Framework x86 this will print

False
True

Note: this bug is harder to reproduce with float on .NET Core because the JIT uses native SSE/SSE2 instruction sets.

jaredpar avatar Jul 12 '24 17:07 jaredpar

Internal ticket NET-1574