Split PrettifyAssignments pass out from ReplaceMethodCallsWithOperators and fix pass ordering.

This simplifies the code; and fixes the assertion during NewtonsoftJson_pcl_debug decompilation.

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]

ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs

@@ -125,12 +125,13 @@ namespace ICSharpCode.Decompiler.CSharp
 
 		List<IAstTransform> astTransforms = new List<IAstTransform> {
 			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

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<ReplaceMethodCallsWithOperators.RestoreOriginalAssignOperatorAnnotation>().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<ReplaceMethodCallsWithOperators.RestoreOriginalAssignOperatorAnnotation>() != 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) {

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
+{
+	/// <summary>
+	/// Simplifies "x = x op y" into "x op= y" where possible.
+	/// </summary>
+	/// <remarks>
+	/// 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").
+	/// </remarks>
+	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<IL.CallInstruction>() == 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);
+		}
+	}
+}

ICSharpCode.Decompiler/CSharp/Transforms/ReplaceMethodCallsWithOperators.cs

@@ -27,7 +27,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 {
 	/// <summary>
 	/// Replaces method calls with the appropriate operator expressions.
-	/// Also simplifies "x = x op y" into "x op= y" where possible.
 	/// </summary>
 	public class ReplaceMethodCallsWithOperators : DepthFirstAstVisitor, IAstTransform
 	{
@@ -197,119 +196,6 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
 			}
 		}
 		
-		/// <summary>
-		/// 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).
-		/// </summary>
-		public class RestoreOriginalAssignOperatorAnnotation
-		{
-			readonly BinaryOperatorExpression binaryOperatorExpression;
-			
-			public RestoreOriginalAssignOperatorAnnotation(BinaryOperatorExpression binaryOperatorExpression)
-			{
-				this.binaryOperatorExpression = binaryOperatorExpression;
-			}
-			
-			public AssignmentExpression Restore(Expression expression)
-			{
-				expression.RemoveAnnotations<RestoreOriginalAssignOperatorAnnotation>();
-				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<IL.CallInstruction>() == 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)),

ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@@ -222,6 +222,7 @@
     <Compile Include="CSharp\Transforms\CombineQueryExpressions.cs" />
     <Compile Include="CSharp\Transforms\IntroduceExtensionMethods.cs" />
     <Compile Include="CSharp\Transforms\IntroduceQueryExpressions.cs" />
+    <Compile Include="CSharp\Transforms\PrettifyAssignments.cs" />
     <Compile Include="CSharp\Transforms\RemoveCLSCompliantAttribute.cs" />
     <Compile Include="CSharp\TranslationContext.cs" />
     <Compile Include="CSharp\TypeSystem\AliasNamespaceReference.cs" />