diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs index e8d209e30..6dfb19368 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/Generics.cs @@ -276,5 +276,14 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty return sizeof(T); } #endif + + public static void Issue1959(int a, int b, int? c) + { + // This line requires parentheses around `a < b` to avoid a grammar ambiguity. + Console.WriteLine("{}, {}", (a < b), a > (c ?? b)); + // But here there's no ambiguity: + Console.WriteLine("{}, {}", a < b, a > b); + Console.WriteLine("{}, {}", a < Environment.GetLogicalDrives().Length, a > (c ?? b)); + } } } diff --git a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs index 174673e13..53b10fa6f 100644 --- a/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs +++ b/ICSharpCode.Decompiler/CSharp/CSharpDecompiler.cs @@ -428,7 +428,10 @@ namespace ICSharpCode.Decompiler.CSharp CancellationToken.ThrowIfCancellationRequested(); transform.Run(rootNode, context); } + CancellationToken.ThrowIfCancellationRequested(); rootNode.AcceptVisitor(new InsertParenthesesVisitor { InsertParenthesesForReadability = true }); + CancellationToken.ThrowIfCancellationRequested(); + GenericGrammarAmbiguityVisitor.ResolveAmbiguities(rootNode); } string SyntaxTreeToString(SyntaxTree syntaxTree) diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs index 1f399b9d9..faa67d21c 100644 --- a/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs +++ b/ICSharpCode.Decompiler/CSharp/OutputVisitor/CSharpOutputVisitor.cs @@ -148,29 +148,6 @@ namespace ICSharpCode.Decompiler.CSharp.OutputVisitor RPar(); } -#if DOTNET35 - void WriteCommaSeparatedList(IEnumerable list) - { - WriteCommaSeparatedList(list.SafeCast()); - } - - void WriteCommaSeparatedList(IEnumerable list) - { - WriteCommaSeparatedList(list.SafeCast()); - } - - void WriteCommaSeparatedListInParenthesis(IEnumerable list, bool spaceWithin) - { - WriteCommaSeparatedListInParenthesis(list.SafeCast(), spaceWithin); - } - - void WriteCommaSeparatedListInParenthesis(IEnumerable list, bool spaceWithin) - { - WriteCommaSeparatedListInParenthesis(list.SafeCast(), spaceWithin); - } - -#endif - protected virtual void WriteCommaSeparatedListInBrackets(IEnumerable list, bool spaceWithin) { WriteToken(Roles.LBracket); diff --git a/ICSharpCode.Decompiler/CSharp/OutputVisitor/GenericGrammarAmbiguityVisitor.cs b/ICSharpCode.Decompiler/CSharp/OutputVisitor/GenericGrammarAmbiguityVisitor.cs new file mode 100644 index 000000000..f621e99b3 --- /dev/null +++ b/ICSharpCode.Decompiler/CSharp/OutputVisitor/GenericGrammarAmbiguityVisitor.cs @@ -0,0 +1,120 @@ +// Copyright (c) 2020 Daniel Grunwald +// +// 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; +using System.Diagnostics; +using System.Linq; +using ICSharpCode.Decompiler.CSharp.Syntax; + +namespace ICSharpCode.Decompiler.CSharp.OutputVisitor +{ + /// + /// Used to test for the "F(G<A,B>(7));" grammar ambiguity. + /// + class GenericGrammarAmbiguityVisitor : DepthFirstAstVisitor + { + /// + /// Resolves ambiguities in the specified syntax tree. + /// This method must be called after the InsertParenthesesVisitor, because the ambiguity depends on whether the + /// final `>` in the possible-type-argument is followed by an opening parenthesis. + /// + public static void ResolveAmbiguities(AstNode rootNode) + { + foreach (var node in rootNode.Descendants.OfType()) { + if (CausesAmbiguityWithGenerics(node)) { + node.ReplaceWith(n => new ParenthesizedExpression(n)); + } + } + } + + public static bool CausesAmbiguityWithGenerics(BinaryOperatorExpression binaryOperatorExpression) + { + if (binaryOperatorExpression.Operator != BinaryOperatorType.LessThan) + return false; + + var v = new GenericGrammarAmbiguityVisitor(); + v.genericNestingLevel = 1; + + for (AstNode node = binaryOperatorExpression.Right; node != null; node = node.GetNextNode()) { + if (node.AcceptVisitor(v)) + return v.ambiguityFound; + } + return false; + } + + int genericNestingLevel; + bool ambiguityFound; + + protected override bool VisitChildren(AstNode node) + { + // unhandled node: probably not syntactically valid in a typename + + // These are preconditions for all recursive Visit() calls. + Debug.Assert(genericNestingLevel > 0); + Debug.Assert(!ambiguityFound); + + // The return value merely indicates whether to stop visiting. + return true; // stop visiting, no ambiguity found + } + + public override bool VisitBinaryOperatorExpression(BinaryOperatorExpression binaryOperatorExpression) + { + if (binaryOperatorExpression.Left.AcceptVisitor(this)) + return true; + Debug.Assert(genericNestingLevel > 0); + switch (binaryOperatorExpression.Operator) { + case BinaryOperatorType.LessThan: + genericNestingLevel += 1; + break; + case BinaryOperatorType.GreaterThan: + genericNestingLevel--; + break; + case BinaryOperatorType.ShiftRight when genericNestingLevel >= 2: + genericNestingLevel -= 2; + break; + default: + return true; // stop visiting, no ambiguity found + } + if (genericNestingLevel == 0) { + // Of the all tokens that might follow `>` and trigger the ambiguity to be resolved in favor of generics, + // `(` is the only one that might start an expression. + ambiguityFound = binaryOperatorExpression.Right is ParenthesizedExpression; + return true; // stop visiting + } + return binaryOperatorExpression.Right.AcceptVisitor(this); + } + + public override bool VisitIdentifierExpression(IdentifierExpression identifierExpression) + { + // identifier could also be valid in a type argument + return false; // keep visiting + } + + public override bool VisitTypeReferenceExpression(TypeReferenceExpression typeReferenceExpression) + { + return false; // keep visiting + } + + public override bool VisitMemberReferenceExpression(MemberReferenceExpression memberReferenceExpression) + { + // MRE could also be valid in a type argument + return memberReferenceExpression.Target.AcceptVisitor(this); + } + + } +} diff --git a/ICSharpCode.Decompiler/DebugInfo/PortablePdbWriter.cs b/ICSharpCode.Decompiler/DebugInfo/PortablePdbWriter.cs index 1ed23bdda..64eda349e 100644 --- a/ICSharpCode.Decompiler/DebugInfo/PortablePdbWriter.cs +++ b/ICSharpCode.Decompiler/DebugInfo/PortablePdbWriter.cs @@ -297,7 +297,6 @@ namespace ICSharpCode.Decompiler.DebugInfo { StringWriter w = new StringWriter(); TokenWriter tokenWriter = new TextWriterTokenWriter(w); - syntaxTree.AcceptVisitor(new InsertParenthesesVisitor { InsertParenthesesForReadability = true }); tokenWriter = TokenWriter.WrapInWriterThatSetsLocationsInAST(tokenWriter); syntaxTree.AcceptVisitor(new CSharpOutputVisitor(tokenWriter, settings.CSharpFormattingOptions)); return w.ToString(); diff --git a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj index 651a2ec30..a28fea867 100644 --- a/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj +++ b/ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj @@ -61,6 +61,7 @@ +