From 1c71f6ad46b333a06ae3f9e88597afec20d144b7 Mon Sep 17 00:00:00 2001 From: Siegfried Pammer Date: Sun, 21 Jul 2024 10:33:09 +0200 Subject: [PATCH] Support concatenation involving arguments of type ReadOnlySpan. --- .../RoundtripAssembly.cs | 3 +- .../TestCases/Pretty/StringInterpolation.cs | 25 +++++++ ICSharpCode.Decompiler/CSharp/CallBuilder.cs | 66 +++++++++++++++++++ .../IL/Transforms/ILInlining.cs | 58 ++++++++++++++-- 4 files changed, 147 insertions(+), 5 deletions(-) diff --git a/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs b/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs index 24d61e081..de1639cf0 100644 --- a/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs +++ b/ICSharpCode.Decompiler.Tests/RoundtripAssembly.cs @@ -29,11 +29,12 @@ using CliWrap; using ICSharpCode.Decompiler.CSharp; using ICSharpCode.Decompiler.CSharp.ProjectDecompiler; using ICSharpCode.Decompiler.Metadata; +using ICSharpCode.Decompiler.Tests; using ICSharpCode.Decompiler.Tests.Helpers; using NUnit.Framework; -namespace ICSharpCode.Decompiler.Tests.Roundtrip +namespace ICSharpCode.Decompiler.Roundtrip { [TestFixture, Parallelizable(ParallelScope.All)] public class RoundtripAssembly diff --git a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/StringInterpolation.cs b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/StringInterpolation.cs index fc667d275..0dbbc9064 100644 --- a/ICSharpCode.Decompiler.Tests/TestCases/Pretty/StringInterpolation.cs +++ b/ICSharpCode.Decompiler.Tests/TestCases/Pretty/StringInterpolation.cs @@ -117,5 +117,30 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty public void RequiresCast(IFormattable value) { } + + public string ConcatStringCharSC(string s, char c) + { + return s + c; + } + + public string ConcatStringCharCS(string s, char c) + { + return c + s; + } + + public string ConcatStringCharSCS(string s, char c) + { + return s + c + s; + } + + public string ConcatStringCharCSS(string s, char c) + { + return c + s + s; + } + + public string ConcatStringCharCSSC(string s, char c) + { + return c + s + s + c; + } } } diff --git a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs index adc8afa71..1926a74dc 100644 --- a/ICSharpCode.Decompiler/CSharp/CallBuilder.cs +++ b/ICSharpCode.Decompiler/CSharp/CallBuilder.cs @@ -223,10 +223,76 @@ namespace ICSharpCode.Decompiler.CSharp valueTupleAssembly: inst.Method.DeclaringType.GetDefinition()?.ParentModule )).WithILInstruction(inst); } + if (settings.StringConcat && IsSpanBasedStringConcat(inst, out var operands)) + { + return BuildStringConcat(inst.Method, operands).WithILInstruction(inst); + } return Build(inst.OpCode, inst.Method, inst.Arguments, constrainedTo: inst.ConstrainedTo) .WithILInstruction(inst); } + private ExpressionWithResolveResult BuildStringConcat(IMethod method, List<(ILInstruction Instruction, KnownTypeCode TypeCode)> operands) + { + IType type = typeSystem.FindType(operands[0].TypeCode); + ExpressionWithResolveResult result = expressionBuilder.Translate(operands[0].Instruction, type).ConvertTo(type, expressionBuilder); + var rr = new MemberResolveResult(null, method); + + for (int i = 1; i < operands.Count; i++) + { + type = typeSystem.FindType(operands[i].TypeCode); + var expr = expressionBuilder.Translate(operands[i].Instruction, type).ConvertTo(type, expressionBuilder); + result = new BinaryOperatorExpression(result.Expression, BinaryOperatorType.Add, expr).WithRR(rr); + } + + return result; + } + + private bool IsSpanBasedStringConcat(CallInstruction call, out List<(ILInstruction, KnownTypeCode)> operands) + { + operands = null; + + if (call.Method is not { Name: "Concat", IsStatic: true }) + { + return false; + } + if (!call.Method.DeclaringType.IsKnownType(KnownTypeCode.String)) + { + return false; + } + + int? firstStringArgumentIndex = null; + operands = new(); + + foreach (var arg in call.Arguments) + { + if (arg is Call opImplicit && IsStringToReadOnlySpanCharImplicitConversion(opImplicit.Method)) + { + firstStringArgumentIndex ??= arg.ChildIndex; + operands.Add((opImplicit.Arguments.Single(), KnownTypeCode.String)); + } + else if (arg is NewObj { Arguments: [AddressOf addressOf] } newObj && ILInlining.IsReadOnlySpanCharCtor(newObj.Method)) + { + operands.Add((addressOf.Value, KnownTypeCode.Char)); + } + else + { + return false; + } + } + + return call.Arguments.Count >= 2 && firstStringArgumentIndex <= 1; + } + + private bool IsStringToReadOnlySpanCharImplicitConversion(IMethod method) + { + return method.IsOperator + && method.Name == "op_Implicit" + && method.Parameters.Count == 1 + && method.ReturnType.IsKnownType(KnownTypeCode.ReadOnlySpanOfT) + && method.ReturnType.TypeArguments[0].IsKnownType(KnownTypeCode.Char) + && method.Parameters[0].Type.IsKnownType(KnownTypeCode.String); + } + public ExpressionWithResolveResult Build(OpCode callOpCode, IMethod method, IReadOnlyList callArguments, IReadOnlyList argumentToParameterMap = null, diff --git a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs index 416972039..49ff62cc2 100644 --- a/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs +++ b/ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs @@ -301,7 +301,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms // Thus, we have to ensure we're operating on an r-value. // Additionally, we cannot inline in cases where the C# compiler prohibits the direct use // of the rvalue (e.g. M(ref (MyStruct)obj); is invalid). - if (IsUsedAsThisPointerInCall(loadInst, out var method, out var constrainedTo) || IsPassedToInParameter(loadInst, out method)) + if (IsUsedAsThisPointerInCall(loadInst, out var method, out var constrainedTo)) { if (options.HasFlag(InliningOptions.Aggressive)) { @@ -326,6 +326,39 @@ namespace ICSharpCode.Decompiler.IL.Transforms throw new InvalidOperationException("invalid expression classification"); } } + else if (IsPassedToReadOnlySpanOfCharCtor(loadInst)) + { + // Always inlining is possible here, because it's an 'in' or 'ref readonly' parameter + // and the C# compiler allows calling it with an rvalue, even though that might produce + // a warning. Note that we don't need to check the expression classification, because + // expressionBuilder.VisitAddressOf will handle creating the copy for us. + // This is necessary, because there are compiler-generated uses of this ctor when + // concatenating a string to a char and our following transforms assume the char is + // already inlined. + return true; + } + else if (IsPassedToInParameter(loadInst)) + { + if (options.HasFlag(InliningOptions.Aggressive)) + { + // Inlining might be required in ctor initializers (see #2714). + // expressionBuilder.VisitAddressOf will handle creating the copy for us. + return true; + } + + switch (ClassifyExpression(inlinedExpression)) + { + case ExpressionClassification.RValue: + // For rvalues passed to in parameters, the C# compiler generates a temporary. + return true; + case ExpressionClassification.MutableLValue: + case ExpressionClassification.ReadonlyLValue: + // For lvalues passed to in parameters, the C# compiler never generates temporaries. + return false; + default: + throw new InvalidOperationException("invalid expression classification"); + } + } else if (IsUsedAsThisPointerInFieldRead(loadInst)) { // mcs generated temporaries for field reads on rvalues (#1555) @@ -415,17 +448,34 @@ namespace ICSharpCode.Decompiler.IL.Transforms return inst != ldloca && inst.Parent is LdObj; } - static bool IsPassedToInParameter(LdLoca ldloca, out IMethod method) + static bool IsPassedToInParameter(LdLoca ldloca) { - method = null; if (ldloca.Parent is not CallInstruction call) { return false; } - method = call.Method; return call.GetParameter(ldloca.ChildIndex)?.ReferenceKind is ReferenceKind.In; } + static bool IsPassedToReadOnlySpanOfCharCtor(LdLoca ldloca) + { + if (ldloca.Parent is not NewObj call) + { + return false; + } + return IsReadOnlySpanCharCtor(call.Method); + } + + internal static bool IsReadOnlySpanCharCtor(IMethod method) + { + return method.IsConstructor + && method.Parameters.Count == 1 + && method.DeclaringType.IsKnownType(KnownTypeCode.ReadOnlySpanOfT) + && method.DeclaringType.TypeArguments[0].IsKnownType(KnownTypeCode.Char) + && method.Parameters[0].Type is ByReferenceType brt + && brt.ElementType.IsKnownType(KnownTypeCode.Char); + } + /// /// Gets whether the instruction, when converted into C#, turns into an l-value that can /// be used to mutate a value-type.