qwertie / ecsharp

Home of LoycCore, the LES language of Loyc trees, the Enhanced C# parser, the LeMP macro preprocessor, and the LLLPG parser generator.
http://ecsharp.net
Other
172 stars 25 forks source link

FPL Abs bug #144

Open ZhangHuan0407 opened 1 month ago

ZhangHuan0407 commented 1 month ago

PL16 value = FPL16.MinValue; FPL16 absValue = value.Abs(); Console.WriteLine($"{value}, {absValue}"); // -140737488355328, -140737488355328 value = new FPL16(-1); absValue = value.Abs(); Console.WriteLine($"{value}, {absValue}"); // -1, 1

long testValue = long.MinValue; Console.WriteLine($"{long.MinValue}, {-testValue}"); // -9223372036854775808, -9223372036854775808 try { Math.Abs(testValue); } catch (Exception ex) { // System.OverflowException: Negating the minimum value of a twos complement number is invalid. Console.WriteLine(ex.ToString()); }

ZhangHuan0407 commented 1 month ago

I think the result of FPL16.Abs cannot be negative. Version: Nuget 30.1.1

// Decompile the code public FPL16 Abs() { return Prescaled((N >= 0) ? N : (-N)); }

// Modified public FPI16 Abs() { Int32 valueN = N; if (valueN == MinValue.N) valueN = MaxValue.N; return Prescaled(valueN >= 0 ? valueN : -valueN); }

// dotnet source code https://referencesource.microsoft.com/#mscorlib/system/random.cs,61 They changed MinValue to MaxValue where they didn't expect an error.

System.Math.Abs throw exception, if value equal long.MinValue.

Console.WriteLine(Math.Abs(float.MinValue)); // 3.4028235E+38

Are fixed point and decimal more similar?

qwertie commented 3 days ago

Hi, sorry for not replying to this.

I suggest adding an extension method for this. I definitely don't want to throw an exception like Math.Abs(int.MinValue) (it's very surprising behavior) so if I were to fix it, I'd have MinValue flip to MaxValue. But I don't even want the performance cost of checking for MinValue either.

ZhangHuan0407 commented 2 days ago

I'm currently clone and modified the repository source code. As described in the license, since I use esharp, I need to publish the modified ecsharp source code when I release the product. (At least 3 months before the product will be released)

I also didn't test the performance... I'm using Unity's IL2CPP, and I'm guessing that a simple comparison of values won't be able to test the performance difference.

    public FPL16 Abs()
    {
        Int64 valueN = N;
        if (valueN == MinValue.N)
            valueN = MaxValue.N;
        return Prescaled(valueN >= 0 ? valueN : -valueN);
    }