Merge pull request #1675 from Jacky720/decompiler-paren-cleanup

Decompiler parenthesis cleanup

UndertaleModLib/Decompiler/DoubleToString.cs

@@ -36,15 +36,17 @@ public static string StringOf(double number)
             if (PredefinedValues.TryGetValue(number, out string res))
                 return res;
 
-            ReadOnlySpan<char> numberAsSpan = number.ToString("G17", CultureInfo.InvariantCulture).AsSpan();      // This can sometimes return a scientific notation
+            ReadOnlySpan<char> numberAsSpan = number.ToString("G16", CultureInfo.InvariantCulture).AsSpan();      // This can sometimes return a scientific notation
             int indexOfE = numberAsSpan.IndexOf("E".AsSpan());
             if (indexOfE < 0)
                 return numberAsSpan.ToString();
 
             // This converts the scientific notation to standard form/fixed point notation
             // As of time of writing this comment, C# does not offer a way to print fixed point notation while preserving precision.
             // You may ask "But why not use F17?". And the answer is, that for everything but R and G, the precision is hard-capped to 15 (according to MSDN: Double.ToString()).
-            // Thus we use G17 to keep our precision, and then manually convert this to fixed point notation.
+            // You may also ask "And G17? What merits the drop to G16?".
+            // This is because G17 is long enough to show IEEE 754 errors, which almost always make code less readable and don't affect recompilation.
+            // Thus we use G16, and then manually convert this to fixed point notation.
             // For anyone unaware of the general algorithm: you get the exponent 'n', then move the decimal point n times to the right if it's positive / left if it's negative.
             ReadOnlySpan<char> exponentAsSpan = numberAsSpan.Slice(indexOfE + 1);
             int exponent = Int32.Parse(exponentAsSpan);

UndertaleModLib/Decompiler/Instructions/Decompiler.AssignmentStatement.cs

@@ -129,7 +129,16 @@ bool checkEqual(ExpressionVar a, ExpressionVar b)
                     }
                 }
             }
-            return String.Format("{0}{1}{2} {3}", varPrefix, varName, context.DecompilingStruct ? ":" : " =", Value.ToString(context));
+
+            // Since parenthesized is now the default output of ExpressionTwo (mainly math operations)
+            // It is specifically safe in variable assignment to not parenthesize this (e.g. "foo = a + b")
+            string stringOfValue;
+            if (Value is ExpressionTwo expressionTwoVal)
+                stringOfValue = expressionTwoVal.ToStringNoParens(context);
+            else
+                stringOfValue = Value.ToString(context);
+
+            return String.Format("{0}{1}{2} {3}", varPrefix, varName, context.DecompilingStruct ? ":" : " =", stringOfValue);
         }
 
         public override Statement CleanStatement(DecompileContext context, BlockHLStatement block)

UndertaleModLib/Decompiler/Instructions/Decompiler.DirectFunctionCall.cs

@@ -86,13 +86,21 @@ cast.Argument is ExpressionConstant constant &&
             }
             else
             {
-                foreach (Expression exp in Arguments)
+                if (Arguments.Count == 1 && Arguments[0] is ExpressionTwo expTwo)
                 {
                     context.currentFunction = this;
-                    if (argumentString.Length > 0)
-                        argumentString.Append(", ");
-                    argumentString.Append(exp.ToString(context));
+                    argumentString.Append(expTwo.ToStringNoParens(context));
+                }
+                else
+                {
+                    foreach (Expression exp in Arguments)
+                    {
+                        context.currentFunction = this;
+                        if (argumentString.Length > 0)
+                            argumentString.Append(", ");
+                        argumentString.Append(exp.ToString(context));
 
+                    }
                 }
                 context.currentFunction = null;
 

UndertaleModLib/Decompiler/Instructions/Decompiler.ExpressionCompare.cs

@@ -29,7 +29,7 @@ public override string ToString(DecompileContext context)
         {
             string arg1 = Argument1.ToString(context);
             string arg2 = Argument2.ToString(context);
-            return String.Format("({0} {1} {2})", arg1, OperationToPrintableString(Opcode), arg2);
+            return String.Format("{0} {1} {2}", arg1, OperationToPrintableString(Opcode), arg2);
         }
 
         public string ToStringWithParen(DecompileContext context)

UndertaleModLib/Decompiler/Instructions/Decompiler.ExpressionTwo.cs

@@ -34,15 +34,74 @@ public override Statement CleanStatement(DecompileContext context, BlockHLStatem
             return this;
         }
 
-        public override string ToString(DecompileContext context)
+        private string AddParensIfNeeded(Expression argument, DecompileContext context, bool isSecond)
+        {
+            string arg = argument.ToString(context);
+            bool needsParens;
+            if (arg[0] != '(' &&
+                argument is not ExpressionConstant &&
+                arg.Contains(' ', StringComparison.InvariantCulture))
+                needsParens = true;
+            else
+                needsParens = false;
+
+
+            if (argument is ExpressionTwo argumentAsBinaryExpression)
+            {
+                int outerPriorityLevel = Opcode switch
+                {
+                    UndertaleInstruction.Opcode.Mul or UndertaleInstruction.Opcode.Div => 2,
+                    UndertaleInstruction.Opcode.Add or UndertaleInstruction.Opcode.Sub => 1,
+                    _ => 0,
+                };
+
+                // First, no parentheses on this type
+                arg = argumentAsBinaryExpression.ToStringNoParens(context);
+
+                int argPriorityLevel = argumentAsBinaryExpression.Opcode switch
+                {
+                    UndertaleInstruction.Opcode.Mul or UndertaleInstruction.Opcode.Div => 2,
+                    UndertaleInstruction.Opcode.Add or UndertaleInstruction.Opcode.Sub => 1,
+                    _ => 0,
+                };
+
+
+                // Suppose we have "(arg1a argOp arg1b) opcode argument2", and are wondering whether the depicted parentheses are needed
+                // If the argument's opcode is more highly-prioritized than our own, such as it being multiplication
+                // while we use addition, then no parentheses are required.
+                // If the argument's opcode doesn't fall into typical math rules (that is, I don't know my full order of operations)
+                // Assume it has lower priority and needs parentheses to clarify.
+                // Parentheses are also not needed for operations of the same level, especially string concatenation.
+                needsParens = (outerPriorityLevel > argPriorityLevel);
+                if (outerPriorityLevel == 0)
+                    needsParens = true; // Better safe than sorry
+
+                // Non-commutative operators may still need parentheses, e.g `a - (b - c)`
+                bool nonCommutative = Opcode is UndertaleInstruction.Opcode.Sub or UndertaleInstruction.Opcode.Div;
+                if (isSecond && nonCommutative && Opcode == argumentAsBinaryExpression.Opcode)
+                    needsParens = true;
+            }
+
+            return (needsParens ? String.Format("({0})", arg) : arg);
+        }
+
+        public string ToStringNoParens(DecompileContext context)
         {
+            string arg1 = AddParensIfNeeded(Argument1, context, false);
+            string arg2 = AddParensIfNeeded(Argument2, context, true);
+
             if (Opcode == UndertaleInstruction.Opcode.Or || Opcode == UndertaleInstruction.Opcode.And)
             {
                 // If both arguments are a boolean type, this is a non-short-circuited logical condition
                 if (Type == UndertaleInstruction.DataType.Boolean && Type2 == UndertaleInstruction.DataType.Boolean)
-                    return String.Format("({0} {1}{1} {2})", Argument1.ToString(context), OperationToPrintableString(Opcode), Argument2.ToString(context));
+                    return String.Format("{0} {1}{1} {2}", arg1, OperationToPrintableString(Opcode), arg2);
             }
-            return String.Format("({0} {1} {2})", Argument1.ToString(context), OperationToPrintableString(Opcode), Argument2.ToString(context));
+            return String.Format("{0} {1} {2}", arg1, OperationToPrintableString(Opcode), arg2);
+        }
+
+        public override string ToString(DecompileContext context)
+        {
+            return String.Format("({0})", ToStringNoParens(context));
         }
 
         internal override AssetIDType DoTypePropagation(DecompileContext context, AssetIDType suggestedType)