From 02d1bfddbfe5b6e0fae6ac43325a1e59fe3a1b7f Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Sat, 7 Aug 2021 15:40:39 +0200 Subject: [PATCH 1/5] Fix reversed output in interpolation concatenation --- .../LocalRewriter_BinaryOperator.cs | 5 ++-- .../Semantic/Semantics/InterpolationTests.cs | 27 +++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs index 538a84e6c6a70..faf6486d16ea4 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs @@ -5,6 +5,7 @@ using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; +using System.Linq; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; @@ -157,7 +158,7 @@ private static ImmutableArray CollectBinaryOperatorInterpolated var partsBuilder = ArrayBuilder.GetInstance(); while (true) { - partsBuilder.AddRange(((BoundInterpolatedString)node.Right).Parts); + partsBuilder.AddRange(((BoundInterpolatedString)node.Right).Parts.Reverse()); if (node.Left is BoundBinaryOperator next) { @@ -165,7 +166,7 @@ private static ImmutableArray CollectBinaryOperatorInterpolated } else { - partsBuilder.AddRange(((BoundInterpolatedString)node.Left).Parts); + partsBuilder.AddRange(((BoundInterpolatedString)node.Left).Parts.Reverse()); break; } } diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs index 04179adf4ec95..a46adfddb32cd 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs @@ -12182,6 +12182,33 @@ .locals init (int V_0, //i1 "); } + [Theory] + [InlineData(@"$""({i1}),"" + $""[{i2}],"" + $""{{{i3}}}""")] + [InlineData(@"($""({i1}),"" + $""[{i2}],"") + $""{{{i3}}}""")] + [InlineData(@"$""({i1}),"" + ($""[{i2}],"" + $""{{{i3}}}"")")] + public void InterpolatedStringsAddedUnderObjectAddition2(string expression) + { + var code = $@" +int i1 = 1; +int i2 = 2; +int i3 = 3; +System.Console.WriteLine({expression});"; + + var comp = CreateCompilation(new[] { code, GetInterpolatedStringHandlerDefinition(includeSpanOverloads: false, useDefaultParameters: false, useBoolReturns: false) }); + + CompileAndVerify(comp, expectedOutput: @" +( +value:1 +), +[ +value:2 +], +{ +value:3 +} +"); + } + [Fact] public void InterpolatedStringsAddedUnderObjectAddition_DefiniteAssignment() { From cbf8f5454a570675a60d35587fb814c4a5eb1ce4 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Sat, 7 Aug 2021 17:15:58 +0200 Subject: [PATCH 2/5] Address feedback --- .../LocalRewriter_BinaryOperator.cs | 12 +++- .../Semantic/Semantics/InterpolationTests.cs | 60 +++++++++++++++++++ 2 files changed, 70 insertions(+), 2 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs index faf6486d16ea4..5ea6f862b651f 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs @@ -158,7 +158,7 @@ private static ImmutableArray CollectBinaryOperatorInterpolated var partsBuilder = ArrayBuilder.GetInstance(); while (true) { - partsBuilder.AddRange(((BoundInterpolatedString)node.Right).Parts.Reverse()); + addReversedParts((BoundInterpolatedString)node.Right); if (node.Left is BoundBinaryOperator next) { @@ -166,7 +166,7 @@ private static ImmutableArray CollectBinaryOperatorInterpolated } else { - partsBuilder.AddRange(((BoundInterpolatedString)node.Left).Parts.Reverse()); + addReversedParts((BoundInterpolatedString)node.Left); break; } } @@ -175,6 +175,14 @@ private static ImmutableArray CollectBinaryOperatorInterpolated ImmutableArray parts = partsBuilder.ToImmutableAndFree(); return parts; + + void addReversedParts(BoundInterpolatedString boundInterpolated) + { + for (int i = boundInterpolated.Parts.Length - 1; i >= 0; i--) + { + partsBuilder.Add(boundInterpolated.Parts[i]); + } + } } private BoundExpression MakeBinaryOperator( diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs index a46adfddb32cd..0e9807facf85d 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs @@ -12209,6 +12209,66 @@ public void InterpolatedStringsAddedUnderObjectAddition2(string expression) "); } + [Fact] + public void InterpolatedStringsAddedUnderObjectAddition3() + { + var code = @" +using System; + +try +{ + string s = string.Empty; + Console.WriteLine($""{s = null}{s.Length}"" + $""""); +} +catch (Exception ex) +{ + Console.WriteLine(ex.ToString()); +} +"; + + var comp = CreateCompilation(new[] { code, GetInterpolatedStringHandlerDefinition(includeSpanOverloads: false, useDefaultParameters: false, useBoolReturns: false) }); + + CompileAndVerify(comp, expectedOutput: @"System.NullReferenceException: Object reference not set to an instance of an object. + at Program.
$(String[] args) +").VerifyIL("", @" +{ + // Code size 64 (0x40) + .maxstack 3 + .locals init (string V_0, //s + System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_1) + .try + { + IL_0000: ldsfld ""string string.Empty"" + IL_0005: stloc.0 + IL_0006: ldc.i4.0 + IL_0007: ldc.i4.2 + IL_0008: newobj ""System.Runtime.CompilerServices.DefaultInterpolatedStringHandler..ctor(int, int)"" + IL_000d: stloc.1 + IL_000e: ldloca.s V_1 + IL_0010: ldnull + IL_0011: dup + IL_0012: stloc.0 + IL_0013: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(string)"" + IL_0018: ldloca.s V_1 + IL_001a: ldloc.0 + IL_001b: callvirt ""int string.Length.get"" + IL_0020: call ""void System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.AppendFormatted(int)"" + IL_0025: ldloca.s V_1 + IL_0027: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" + IL_002c: call ""void System.Console.WriteLine(string)"" + IL_0031: leave.s IL_003f + } + catch System.Exception + { + IL_0033: callvirt ""string object.ToString()"" + IL_0038: call ""void System.Console.WriteLine(string)"" + IL_003d: leave.s IL_003f + } + IL_003f: ret +} +"); + } + [Fact] public void InterpolatedStringsAddedUnderObjectAddition_DefiniteAssignment() { From 4db1ed1a08b774366b40370c0cf57e021fbad23b Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Sat, 7 Aug 2021 17:51:51 +0200 Subject: [PATCH 3/5] Address feedback --- .../LocalRewriter/LocalRewriter_BinaryOperator.cs | 2 -- .../Test/Semantic/Semantics/InterpolationTests.cs | 10 ++++++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs index 5ea6f862b651f..7f07afe504fbc 100644 --- a/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs +++ b/src/Compilers/CSharp/Portable/Lowering/LocalRewriter/LocalRewriter_BinaryOperator.cs @@ -2,10 +2,8 @@ // The .NET Foundation licenses this file to you under the MIT license. // See the LICENSE file in the project root for more information. -using System.Collections.Generic; using System.Collections.Immutable; using System.Diagnostics; -using System.Linq; using Microsoft.CodeAnalysis.CSharp.Symbols; using Microsoft.CodeAnalysis.PooledObjects; using Roslyn.Utilities; diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs index 0e9807facf85d..7ff0168f8fc53 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs @@ -12213,11 +12213,13 @@ public void InterpolatedStringsAddedUnderObjectAddition2(string expression) public void InterpolatedStringsAddedUnderObjectAddition3() { var code = @" +#nullable enable + using System; try { - string s = string.Empty; + var s = string.Empty; Console.WriteLine($""{s = null}{s.Length}"" + $""""); } catch (Exception ex) @@ -12266,7 +12268,11 @@ .locals init (string V_0, //s } IL_003f: ret } -"); +").VerifyDiagnostics( + // (9,36): warning CS8602: Dereference of a possibly null reference. + // Console.WriteLine($"{s = null}{s.Length}" + $""); + Diagnostic(ErrorCode.WRN_NullReferenceReceiver, "s").WithLocation(9, 36) + ); } [Fact] From c8052f518deb021b0f398defb18548d9aabd08c1 Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Sun, 8 Aug 2021 12:36:38 +0200 Subject: [PATCH 4/5] Fix test --- .../CSharp/Test/Semantic/Semantics/InterpolationTests.cs | 8 +++----- 1 file changed, 3 insertions(+), 5 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs index 7ff0168f8fc53..10a365de0f32d 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs @@ -12224,15 +12224,13 @@ public void InterpolatedStringsAddedUnderObjectAddition3() } catch (Exception ex) { - Console.WriteLine(ex.ToString()); + Console.WriteLine(ex.Message); } "; var comp = CreateCompilation(new[] { code, GetInterpolatedStringHandlerDefinition(includeSpanOverloads: false, useDefaultParameters: false, useBoolReturns: false) }); - CompileAndVerify(comp, expectedOutput: @"System.NullReferenceException: Object reference not set to an instance of an object. - at Program.
$(String[] args) -").VerifyIL("", @" + CompileAndVerify(comp, expectedOutput: "Object reference not set to an instance of an object.").VerifyIL("", @" { // Code size 64 (0x40) .maxstack 3 @@ -12262,7 +12260,7 @@ .locals init (string V_0, //s } catch System.Exception { - IL_0033: callvirt ""string object.ToString()"" + IL_0033: callvirt ""string System.Exception.Message.get"" IL_0038: call ""void System.Console.WriteLine(string)"" IL_003d: leave.s IL_003f } From dfef97953c0a4f36f6235daea89cc39b70b811eb Mon Sep 17 00:00:00 2001 From: Youssef1313 Date: Tue, 10 Aug 2021 11:39:35 +0200 Subject: [PATCH 5/5] fix test --- .../Semantic/Semantics/InterpolationTests.cs | 21 ++++++++++--------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs index 10a365de0f32d..5e1f7a5b2dc62 100644 --- a/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs +++ b/src/Compilers/CSharp/Test/Semantic/Semantics/InterpolationTests.cs @@ -12222,17 +12222,17 @@ public void InterpolatedStringsAddedUnderObjectAddition3() var s = string.Empty; Console.WriteLine($""{s = null}{s.Length}"" + $""""); } -catch (Exception ex) +catch (NullReferenceException) { - Console.WriteLine(ex.Message); + Console.WriteLine(""Null reference exception caught.""); } "; var comp = CreateCompilation(new[] { code, GetInterpolatedStringHandlerDefinition(includeSpanOverloads: false, useDefaultParameters: false, useBoolReturns: false) }); - CompileAndVerify(comp, expectedOutput: "Object reference not set to an instance of an object.").VerifyIL("", @" + CompileAndVerify(comp, expectedOutput: "Null reference exception caught.").VerifyIL("", @" { - // Code size 64 (0x40) + // Code size 65 (0x41) .maxstack 3 .locals init (string V_0, //s System.Runtime.CompilerServices.DefaultInterpolatedStringHandler V_1) @@ -12256,15 +12256,16 @@ .locals init (string V_0, //s IL_0025: ldloca.s V_1 IL_0027: call ""string System.Runtime.CompilerServices.DefaultInterpolatedStringHandler.ToStringAndClear()"" IL_002c: call ""void System.Console.WriteLine(string)"" - IL_0031: leave.s IL_003f + IL_0031: leave.s IL_0040 } - catch System.Exception + catch System.NullReferenceException { - IL_0033: callvirt ""string System.Exception.Message.get"" - IL_0038: call ""void System.Console.WriteLine(string)"" - IL_003d: leave.s IL_003f + IL_0033: pop + IL_0034: ldstr ""Null reference exception caught."" + IL_0039: call ""void System.Console.WriteLine(string)"" + IL_003e: leave.s IL_0040 } - IL_003f: ret + IL_0040: ret } ").VerifyDiagnostics( // (9,36): warning CS8602: Dereference of a possibly null reference.