diff --git a/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs b/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs index daae52821..c52969a05 100644 --- a/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs +++ b/ICSharpCode.Decompiler/CSharp/Syntax/SyntaxExtensions.cs @@ -61,6 +61,37 @@ namespace ICSharpCode.Decompiler.CSharp.Syntax return (Statement?)next; } + /// + /// The first statement of that is not an + /// , or null if there is none. + /// + /// + /// An empty statement is either a stray ';' or a placeholder carrying a comment the + /// decompiler emitted (a warning, an unconvertible block, a "try-fault" marker, ...). + /// Neither is a statement in the sense a transform matching a statement sequence means, + /// so every such transform has to look past them or it silently stops recognizing its + /// pattern as soon as one of those comments lands in the middle of the sequence. + /// + public static Statement? GetFirstNonEmptyStatementOrDefault(this AstNodeCollection statements) + { + return statements.FirstOrNull(statement => statement is not EmptyStatement); + } + + /// + /// The next statement after that is not an + /// , or null if there is none. + /// + /// + /// See for why the skip is needed. + /// + public static Statement? GetNextNonEmptyStatement(this Statement statement) + { + var next = statement.GetNextStatement(); + while (next is EmptyStatement) + next = next.GetNextStatement(); + return next; + } + public static bool IsArgList(this AstType? type) { var simpleType = type as SimpleType; diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs b/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs index 73d62258c..99e49bde0 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/FlattenSwitchBlocks.cs @@ -33,10 +33,11 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { foreach (var switchSection in rootNode.Descendants.OfType()) { - if (switchSection.Statements.Count != 1) + var onlyStatement = switchSection.Statements.GetFirstNonEmptyStatementOrDefault(); + if (onlyStatement == null || onlyStatement.GetNextNonEmptyStatement() != null) continue; - var blockStatement = switchSection.Statements.First() as BlockStatement; + var blockStatement = onlyStatement as BlockStatement; if (blockStatement == null || blockStatement.Statements.Any(ContainsLocalDeclaration)) continue; diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs index b5a40d081..e82fb461c 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs @@ -197,7 +197,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (!m1.Success) return null; var variable = m1.Get("variable").Single().GetILVariable(); - AstNode? next = node.NextSibling; + AstNode? next = node.GetNextNonEmptyStatement(); if (next == null) return null; if (next is ForStatement forStatement && ForStatementUsesVariable(forStatement, variable)) @@ -598,7 +598,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms Match m = default(Match); while (i < upperBounds.Length && MatchLowerBound(i, out var indexVariable, collection, stmt)) { - m = forOnArrayMultiDimPattern.Match(stmt.GetNextStatement()); + m = forOnArrayMultiDimPattern.Match(stmt.GetNextNonEmptyStatement()); if (!m.Success) return false; var upperBound = m.Get("upperBoundVariable").Single().GetILVariable(); @@ -654,7 +654,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (!int.TryParse(m.Get("index").Single().Value?.ToString() ?? "", out int index) || index != i) break; upperBounds[i] = m.Get("variable").Single().GetILVariable()!; - stmt = stmt.GetNextStatement(); + stmt = stmt.GetNextNonEmptyStatement(); i++; } while (stmt != null && upperBounds != null && i < upperBounds.Length); @@ -664,7 +664,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms return null; statementsToDelete.Add(stmt); // The matched multi-dimensional foreach pattern guarantees a statement after stmt. - statementsToDelete.Add(stmt.GetNextStatement()!); + statementsToDelete.Add(stmt.GetNextNonEmptyStatement()!); var itemVariable = foreachVariable.GetILVariable(); if (itemVariable == null || !itemVariable.IsSingleDefinition || (itemVariable.Kind != IL.VariableKind.Local && itemVariable.Kind != IL.VariableKind.StackSlot) @@ -1261,34 +1261,79 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms #endregion #region Destructor - static readonly BlockStatement destructorBodyPattern = new BlockStatement { - new TryCatchStatement { - TryBlock = new AnyNode("body"), - FinallyBlock = new BlockStatement { - new InvocationExpression(new MemberReferenceExpression(new BaseReferenceExpression(), "Finalize")) - } - } + static readonly TryCatchStatement destructorTryFinallyPattern = new TryCatchStatement { + TryBlock = new AnyNode("body"), + FinallyBlock = new AnyNode("finallyBlock") }; + static readonly Statement baseFinalizeCallPattern = new ExpressionStatement( + new InvocationExpression(new MemberReferenceExpression(new BaseReferenceExpression(), "Finalize"))); + static readonly MethodDeclaration destructorPattern = new MethodDeclaration { Attributes = { new Repeat(new AnyNode()) }, Modifiers = Modifiers.Any, ReturnType = new PrimitiveType("void"), Name = "Finalize", - Body = destructorBodyPattern + Body = new AnyNode() }; + /// + /// Matches the body a compiler emits for a destructor - a single try statement whose + /// finally block does nothing but call base.Finalize() - and returns the try block + /// holding the user-written code, or null if has another + /// shape. Comment placeholders around the two statements are skipped: leaving the method + /// in its "override Finalize" shape over a decompiler warning produces output that does + /// not compile (CS0249). + /// + static BlockStatement? MatchDestructorBody(BlockStatement body) + { + var statement = body.Statements.GetFirstNonEmptyStatementOrDefault(); + if (statement is not TryCatchStatement || statement.GetNextNonEmptyStatement() != null) + return null; + Match m = destructorTryFinallyPattern.Match(statement); + if (!m.Success) + return null; + var finalizeCall = m.Get("finallyBlock").Single() + .Statements.GetFirstNonEmptyStatementOrDefault(); + if (finalizeCall == null || finalizeCall.GetNextNonEmptyStatement() != null + || !baseFinalizeCallPattern.IsMatch(finalizeCall)) + { + return null; + } + return m.Get("body").Single(); + } + + /// + /// Moves the comment placeholders of to the front of + /// , which replaces it. They describe the member, so dropping + /// them with the body they happen to sit in would lose a decompiler warning. + /// + static void MovePlaceholderComments(BlockStatement oldBody, BlockStatement newBody) + { + var anchor = newBody.Statements.FirstOrNull(); + foreach (var placeholder in oldBody.Statements.OfType().ToList()) + { + placeholder.Detach(); + if (anchor != null) + newBody.Statements.InsertBefore(anchor, placeholder); + else + newBody.Statements.Add(placeholder); + } + } + DestructorDeclaration? TransformDestructor(MethodDeclaration methodDef) { Match m = destructorPattern.Match(methodDef); - if (m.Success) + if (m.Success && methodDef.Body is BlockStatement oldBody + && MatchDestructorBody(oldBody) is BlockStatement tryBlock) { context.Step("Convert Finalize method to destructor", methodDef); DestructorDeclaration dd = new DestructorDeclaration(); methodDef.Attributes.MoveTo(dd.Attributes); dd.CopyAnnotationsFrom(methodDef); dd.Modifiers = methodDef.Modifiers & ~(Modifiers.Protected | Modifiers.Override); - dd.Body = m.Get("body").Single().Detach(); + MovePlaceholderComments(oldBody, tryBlock); + dd.Body = tryBlock.Detach(); // A destructor only appears inside a type declaration, so the context tracker // has an enclosing type at this point. dd.Name = currentTypeDefinition!.Name; @@ -1301,11 +1346,12 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms DestructorDeclaration? TransformDestructorBody(DestructorDeclaration dtorDef) { - Match m = destructorBodyPattern.Match(dtorDef.Body); - if (m.Success) + if (dtorDef.Body is BlockStatement oldBody + && MatchDestructorBody(oldBody) is BlockStatement tryBlock) { context.Step("Simplify destructor body", dtorDef); - dtorDef.Body = m.Get("body").Single().Detach(); + MovePlaceholderComments(oldBody, tryBlock); + dtorDef.Body = tryBlock.Detach(); return dtorDef; } return null; @@ -1455,7 +1501,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (!context.Settings.UseEnhancedUsing) return usingStatement; - if (usingStatement.GetNextStatement() != null || !(usingStatement.Parent is BlockStatement)) + if (usingStatement.GetNextNonEmptyStatement() != null || !(usingStatement.Parent is BlockStatement)) return usingStatement; if (!(usingStatement.ResourceAcquisition is VariableDeclarationStatement)) diff --git a/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs b/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs index b7d379594..b287b541c 100644 --- a/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs +++ b/ICSharpCode.Decompiler/CSharp/Transforms/TransformFieldAndConstructorInitializers.cs @@ -130,7 +130,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms bool skippedStmts = false; Statement? stmt; - for (stmt = ctor.Body?.Statements.FirstOrDefault(); stmt != null; stmt = stmt.GetNextStatement()) + for (stmt = ctor.Body?.Statements.GetFirstNonEmptyStatementOrDefault(); stmt != null; stmt = stmt.GetNextNonEmptyStatement()) { var m = memberInitializerPattern.Match(stmt); if (!m.Success) @@ -178,7 +178,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms : ThisCallClassPattern.Match(stmt); if (m.Success) { - sequence.CoversFullBody = stmt.GetNextStatement() == null; + sequence.CoversFullBody = stmt.GetNextNonEmptyStatement() == null; } } } @@ -228,7 +228,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms if (ctor.Body is null) return false; var stmts = ctor.Body.Statements; - var otherStmt = stmts.FirstOrDefault(); + var otherStmt = stmts.GetFirstNonEmptyStatementOrDefault(); foreach (var (stmt, member, initializer, _) in Statements) { var m = memberInitializerPattern.Match(otherStmt); @@ -249,7 +249,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms StatementToOtherCtorsMap[stmt] = list; } list.Add((otherStmt, otherInitializer)); - otherStmt = otherStmt.GetNextStatement(); + otherStmt = otherStmt.GetNextNonEmptyStatement(); } return true; } @@ -369,7 +369,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms else { // find this-ctor call - var stmt = ctor.Body?.Statements.FirstOrDefault(); + var stmt = ctor.Body?.Statements.GetFirstNonEmptyStatementOrDefault(); var m = ctorMethod.DeclaringType.Kind == TypeKind.Struct ? ThisCallStructPattern.Match(stmt) : ThisCallClassPattern.Match(stmt); @@ -527,7 +527,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms { if (constructorDeclaration.Body is null) return false; - Statement stmt = constructorDeclaration.Body.Statements.FirstOrDefault()!; + Statement stmt = constructorDeclaration.Body.Statements.GetFirstNonEmptyStatementOrDefault()!; var isValueType = ctorMethod.DeclaringType.Kind == TypeKind.Struct; // value types may omit the constructor initializer completely