From 905cb0f388d7f3f1ea89f638e3db3a3b300bb687 Mon Sep 17 00:00:00 2001 From: Daniel Grunwald Date: Tue, 12 Sep 2017 00:06:04 +0200 Subject: [PATCH] Split PrettifyAssignments pass out from ReplaceMethodCallsWithOperators and fix pass ordering. This simplifies the code; and fixes the assertion during NewtonsoftJson_pcl_debug decompilation. --- .../RoundtripAssembly.cs | 8 +- .../CSharp/CSharpDecompiler.cs | 3 +- .../CSharp/Transforms/AddCheckedBlocks.cs | 34 ----- .../CSharp/Transforms/PrettifyAssignments.cs | 123 ++++++++++++++++++ .../ReplaceMethodCallsWithOperators.cs | 114 ---------------- .../ICSharpCode.Decompiler.csproj | 1 + 6 files changed, 132 insertions(+), 151 deletions(-) create mode 100644 ICSharpCode.Decompiler/CSharp/Transforms/PrettifyAssignments.cs diff --git a/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs b/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs index 60f785f70..6bfee5834 100644 --- a/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs +++ b/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs @@ -47,10 +47,14 @@ namespace ICSharpCode.Decompiler.Tests RunWithTest("Newtonsoft.Json-net45", "Newtonsoft.Json.dll", "Newtonsoft.Json.Tests.dll"); } - [Test, Ignore("Do not run on build server")] + [Test] public void NewtonsoftJson_pcl_debug() { - RunWithTest("Newtonsoft.Json-pcl-debug", "Newtonsoft.Json.dll", "Newtonsoft.Json.Tests.dll"); + try { + RunWithTest("Newtonsoft.Json-pcl-debug", "Newtonsoft.Json.dll", "Newtonsoft.Json.Tests.dll"); + } catch (CompilationFailedException) { + Assert.Ignore("Cannot yet re-compile PCL projects."); + } } [Test] diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index 48c6bf33e..7eee97087 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -125,12 +125,13 @@ namespace ICSharpCode.Decompiler.CSharp List astTransforms = new List { new PatternStatementTransform(), - new ReplaceMethodCallsWithOperators(), + new ReplaceMethodCallsWithOperators(), // must run before DeclareVariables.EnsureExpressionStatementsAreValid new IntroduceUnsafeModifier(), new AddCheckedBlocks(), new DeclareVariables(), // should run after most transforms that modify statements new ConvertConstructorCallIntoInitializer(), // must run after DeclareVariables new DecimalConstantTransform(), + new PrettifyAssignments(), // must run after DeclareVariables new IntroduceUsingDeclarations(), new IntroduceExtensionMethods(), // must run after IntroduceUsingDeclarations new IntroduceQueryExpressions(), // must run after IntroduceExtensionMethods diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs b/ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs index c0ee1c56a..856c648ce 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/AddCheckedBlocks.cs @@ -165,28 +165,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } } - class ConvertCompoundAssignment : InsertedNode - { - readonly Expression expression; - readonly bool isChecked; - - public ConvertCompoundAssignment(Expression expression, bool isChecked) - { - this.expression = expression; - this.isChecked = isChecked; - } - - public override void Insert() - { - AssignmentExpression assign = expression.Annotation().Restore(expression); - expression.ReplaceWith(assign); - if (isChecked) - assign.Right = new CheckedExpression { Expression = assign.Right.Detach() }; - else - assign.Right = new UncheckedExpression { Expression = assign.Right.Detach() }; - } - } - class InsertedBlock : InsertedNode { readonly Statement firstStatement; // inclusive @@ -344,18 +322,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms // Embed this node in an checked/unchecked expression: if (expr.Parent is ExpressionStatement) { // We cannot use checked/unchecked for top-level-expressions. - // However, we could try converting a compound assignment (checked(a+=b);) or unary operator (checked(a++);) - // back to its old form. - if (expr.Annotation() != null) { - // We use '<' so that expressions are introduced on the deepest level possible (goal 3) - if (result.CostInCheckedContext + new Cost(1, 1) < result.CostInUncheckedContext) { - result.CostInUncheckedContext = result.CostInCheckedContext + new Cost(1, 1); - result.NodesToInsertInUncheckedContext = result.NodesToInsertInCheckedContext + new ConvertCompoundAssignment(expr, true); - } else if (result.CostInUncheckedContext + new Cost(1, 1) < result.CostInCheckedContext) { - result.CostInCheckedContext = result.CostInUncheckedContext + new Cost(1, 1); - result.NodesToInsertInCheckedContext = result.NodesToInsertInUncheckedContext + new ConvertCompoundAssignment(expr, false); - } - } } else if (expr.Role.IsValid(Expression.Null)) { // We use '<' so that expressions are introduced on the deepest level possible (goal 3) if (result.CostInCheckedContext + new Cost(0, 1) < result.CostInUncheckedContext) { diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PrettifyAssignments.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PrettifyAssignments.cs new file mode 100644 index 000000000..f72e2994f --- /dev/null +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PrettifyAssignments.cs @@ -0,0 +1,123 @@ +// Copyright (c) 2011 AlphaSierraPapa for the SharpDevelop Team +// +// Permission is hereby granted, free of charge, to any person obtaining a copy of this +// software and associated documentation files (the "Software"), to deal in the Software +// without restriction, including without limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons +// to whom the Software is furnished to do so, subject to the following conditions: +// +// The above copyright notice and this permission notice shall be included in all copies or +// substantial portions of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, +// INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR +// PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE +// FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR +// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +using System.Linq; +using ICSharpCode.Decompiler.CSharp.Syntax; +using ICSharpCode.Decompiler.CSharp.Syntax.PatternMatching; + +namespace ICSharpCode.Decompiler.CSharp.Transforms +{ + /// + /// Simplifies "x = x op y" into "x op= y" where possible. + /// + /// + /// Because the two "x" in "x = x op y" may refer to different ILVariables, + /// this transform must run after DeclareVariables. + /// + /// It must also run after ReplaceMethodCallsWithOperators (so that it can work for custom operator, too); + /// and after AddCheckedBlocks (because "for (;; x = unchecked(x op y))" cannot be transformed into "x += y"). + /// + class PrettifyAssignments : DepthFirstAstVisitor, IAstTransform + { + public override void VisitAssignmentExpression(AssignmentExpression assignment) + { + base.VisitAssignmentExpression(assignment); + // Combine "x = x op y" into "x op= y" + BinaryOperatorExpression binary = assignment.Right as BinaryOperatorExpression; + if (binary != null && assignment.Operator == AssignmentOperatorType.Assign) { + if (CanConvertToCompoundAssignment(assignment.Left) && assignment.Left.IsMatch(binary.Left)) { + assignment.Operator = GetAssignmentOperatorForBinaryOperator(binary.Operator); + if (assignment.Operator != AssignmentOperatorType.Assign) { + // If we found a shorter operator, get rid of the BinaryOperatorExpression: + assignment.CopyAnnotationsFrom(binary); + assignment.Right = binary.Right; + } + } + } + // TODO: context.Settings.IntroduceIncrementAndDecrement + if (assignment.Operator == AssignmentOperatorType.Add || assignment.Operator == AssignmentOperatorType.Subtract) { + // detect increment/decrement + if (assignment.Right.IsMatch(new PrimitiveExpression(1))) { + // only if it's not a custom operator + if (assignment.Annotation() == null) { + UnaryOperatorType type; + // When the parent is an expression statement, pre- or post-increment doesn't matter; + // so we can pick post-increment which is more commonly used (for (int i = 0; i < x; i++)) + if (assignment.Parent is ExpressionStatement) + type = (assignment.Operator == AssignmentOperatorType.Add) ? UnaryOperatorType.PostIncrement : UnaryOperatorType.PostDecrement; + else + type = (assignment.Operator == AssignmentOperatorType.Add) ? UnaryOperatorType.Increment : UnaryOperatorType.Decrement; + assignment.ReplaceWith(new UnaryOperatorExpression(type, assignment.Left.Detach()).CopyAnnotationsFrom(assignment)); + } + } + } + } + + public static AssignmentOperatorType GetAssignmentOperatorForBinaryOperator(BinaryOperatorType bop) + { + switch (bop) { + case BinaryOperatorType.Add: + return AssignmentOperatorType.Add; + case BinaryOperatorType.Subtract: + return AssignmentOperatorType.Subtract; + case BinaryOperatorType.Multiply: + return AssignmentOperatorType.Multiply; + case BinaryOperatorType.Divide: + return AssignmentOperatorType.Divide; + case BinaryOperatorType.Modulus: + return AssignmentOperatorType.Modulus; + case BinaryOperatorType.ShiftLeft: + return AssignmentOperatorType.ShiftLeft; + case BinaryOperatorType.ShiftRight: + return AssignmentOperatorType.ShiftRight; + case BinaryOperatorType.BitwiseAnd: + return AssignmentOperatorType.BitwiseAnd; + case BinaryOperatorType.BitwiseOr: + return AssignmentOperatorType.BitwiseOr; + case BinaryOperatorType.ExclusiveOr: + return AssignmentOperatorType.ExclusiveOr; + default: + return AssignmentOperatorType.Assign; + } + } + + static bool CanConvertToCompoundAssignment(Expression left) + { + MemberReferenceExpression mre = left as MemberReferenceExpression; + if (mre != null) + return IsWithoutSideEffects(mre.Target); + IndexerExpression ie = left as IndexerExpression; + if (ie != null) + return IsWithoutSideEffects(ie.Target) && ie.Arguments.All(IsWithoutSideEffects); + UnaryOperatorExpression uoe = left as UnaryOperatorExpression; + if (uoe != null && uoe.Operator == UnaryOperatorType.Dereference) + return IsWithoutSideEffects(uoe.Expression); + return IsWithoutSideEffects(left); + } + + static bool IsWithoutSideEffects(Expression left) + { + return left is ThisReferenceExpression || left is IdentifierExpression || left is TypeReferenceExpression || left is BaseReferenceExpression; + } + + void IAstTransform.Run(AstNode node, TransformContext context) + { + node.AcceptVisitor(this); + } + } +} diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/ReplaceMethodCallsWithOperators.cs b/ICSharpCode.Decompiler/CSharp/Transforms/ReplaceMethodCallsWithOperators.cs index 5ce5ff332..1fb30722c 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/ReplaceMethodCallsWithOperators.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/ReplaceMethodCallsWithOperators.cs @@ -27,7 +27,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { /// /// Replaces method calls with the appropriate operator expressions. - /// Also simplifies "x = x op y" into "x op= y" where possible. /// public class ReplaceMethodCallsWithOperators : DepthFirstAstVisitor, IAstTransform { @@ -197,119 +196,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms } } - /// - /// This annotation is used to allow later pipeline steps to convert a compound assignment "a += 2;" or - /// increment operator "a++;" back to the original "a = a + 2;". - /// This is sometimes necessary when the checked/unchecked semantics cannot be guaranteed otherwise - /// (see CheckedUnchecked.ForWithCheckedInitializerAndUncheckedIterator test). - /// - public class RestoreOriginalAssignOperatorAnnotation - { - readonly BinaryOperatorExpression binaryOperatorExpression; - - public RestoreOriginalAssignOperatorAnnotation(BinaryOperatorExpression binaryOperatorExpression) - { - this.binaryOperatorExpression = binaryOperatorExpression; - } - - public AssignmentExpression Restore(Expression expression) - { - expression.RemoveAnnotations(); - AssignmentExpression assign = expression as AssignmentExpression; - if (assign == null) { - UnaryOperatorExpression uoe = (UnaryOperatorExpression)expression; - assign = new AssignmentExpression(uoe.Expression.Detach(), new PrimitiveExpression(1)); - } else { - assign.Operator = AssignmentOperatorType.Assign; - } - binaryOperatorExpression.Right = assign.Right.Detach(); - assign.Right = binaryOperatorExpression; - return assign; - } - } - - public override void VisitAssignmentExpression(AssignmentExpression assignment) - { - base.VisitAssignmentExpression(assignment); - // Combine "x = x op y" into "x op= y" - BinaryOperatorExpression binary = assignment.Right as BinaryOperatorExpression; - if (binary != null && assignment.Operator == AssignmentOperatorType.Assign) { - if (CanConvertToCompoundAssignment(assignment.Left) && assignment.Left.IsMatch(binary.Left)) { - assignment.Operator = GetAssignmentOperatorForBinaryOperator(binary.Operator); - if (assignment.Operator != AssignmentOperatorType.Assign) { - // If we found a shorter operator, get rid of the BinaryOperatorExpression: - assignment.CopyAnnotationsFrom(binary); - assignment.Right = binary.Right; - assignment.AddAnnotation(new RestoreOriginalAssignOperatorAnnotation(binary)); - } - } - } - // TODO: context.Settings.IntroduceIncrementAndDecrement - if (assignment.Operator == AssignmentOperatorType.Add || assignment.Operator == AssignmentOperatorType.Subtract) { - // detect increment/decrement - if (assignment.Right.IsMatch(new PrimitiveExpression(1))) { - // only if it's not a custom operator - if (assignment.Annotation() == null) { - UnaryOperatorType type; - // When the parent is an expression statement, pre- or post-increment doesn't matter; - // so we can pick post-increment which is more commonly used (for (int i = 0; i < x; i++)) - if (assignment.Parent is ExpressionStatement) - type = (assignment.Operator == AssignmentOperatorType.Add) ? UnaryOperatorType.PostIncrement : UnaryOperatorType.PostDecrement; - else - type = (assignment.Operator == AssignmentOperatorType.Add) ? UnaryOperatorType.Increment : UnaryOperatorType.Decrement; - assignment.ReplaceWith(new UnaryOperatorExpression(type, assignment.Left.Detach()).CopyAnnotationsFrom(assignment)); - } - } - } - } - - public static AssignmentOperatorType GetAssignmentOperatorForBinaryOperator(BinaryOperatorType bop) - { - switch (bop) { - case BinaryOperatorType.Add: - return AssignmentOperatorType.Add; - case BinaryOperatorType.Subtract: - return AssignmentOperatorType.Subtract; - case BinaryOperatorType.Multiply: - return AssignmentOperatorType.Multiply; - case BinaryOperatorType.Divide: - return AssignmentOperatorType.Divide; - case BinaryOperatorType.Modulus: - return AssignmentOperatorType.Modulus; - case BinaryOperatorType.ShiftLeft: - return AssignmentOperatorType.ShiftLeft; - case BinaryOperatorType.ShiftRight: - return AssignmentOperatorType.ShiftRight; - case BinaryOperatorType.BitwiseAnd: - return AssignmentOperatorType.BitwiseAnd; - case BinaryOperatorType.BitwiseOr: - return AssignmentOperatorType.BitwiseOr; - case BinaryOperatorType.ExclusiveOr: - return AssignmentOperatorType.ExclusiveOr; - default: - return AssignmentOperatorType.Assign; - } - } - - static bool CanConvertToCompoundAssignment(Expression left) - { - MemberReferenceExpression mre = left as MemberReferenceExpression; - if (mre != null) - return IsWithoutSideEffects(mre.Target); - IndexerExpression ie = left as IndexerExpression; - if (ie != null) - return IsWithoutSideEffects(ie.Target) && ie.Arguments.All(IsWithoutSideEffects); - UnaryOperatorExpression uoe = left as UnaryOperatorExpression; - if (uoe != null && uoe.Operator == UnaryOperatorType.Dereference) - return IsWithoutSideEffects(uoe.Expression); - return IsWithoutSideEffects(left); - } - - static bool IsWithoutSideEffects(Expression left) - { - return left is ThisReferenceExpression || left is IdentifierExpression || left is TypeReferenceExpression || left is BaseReferenceExpression; - } - static readonly Expression getMethodOrConstructorFromHandlePattern = new CastExpression(new Choice { new TypePattern(typeof(MethodInfo)), diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 3977ac0b9..6a65f9788 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -222,6 +222,7 @@ +