Browse Source

Fix #2714: Force inlining of call targets in ctor initializers.

pull/2726/head
Siegfried Pammer 3 years ago
parent
commit
702a7da2c3
  1. 14
      ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs
  2. 13
      ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs
  3. 62
      ICSharpCode.Decompiler/CSharp/Resolver/CastResolveResult.cs
  4. 1
      ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj
  5. 25
      ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

14
ICSharpCode.Decompiler.Tests/TestCases/Pretty/ConstructorInitializers.cs

@ -16,6 +16,8 @@
// OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER // OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER
// DEALINGS IN THE SOFTWARE. // DEALINGS IN THE SOFTWARE.
using System;
namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
{ {
public class ConstructorInitializers public class ConstructorInitializers
@ -55,6 +57,18 @@ namespace ICSharpCode.Decompiler.Tests.TestCases.Pretty
} }
} }
public class MethodCallInCtorInit
{
public MethodCallInCtorInit(ConsoleKey key)
: this(((int)key).ToString())
{
}
public MethodCallInCtorInit(string s)
{
}
}
public struct SimpleStruct public struct SimpleStruct
{ {
public int Field1; public int Field1;

13
ICSharpCode.Decompiler/CSharp/ExpressionBuilder.cs

@ -29,6 +29,7 @@ using ICSharpCode.Decompiler.CSharp.Syntax;
using ICSharpCode.Decompiler.CSharp.Transforms; using ICSharpCode.Decompiler.CSharp.Transforms;
using ICSharpCode.Decompiler.CSharp.TypeSystem; using ICSharpCode.Decompiler.CSharp.TypeSystem;
using ICSharpCode.Decompiler.IL; using ICSharpCode.Decompiler.IL;
using ICSharpCode.Decompiler.IL.Transforms;
using ICSharpCode.Decompiler.Semantics; using ICSharpCode.Decompiler.Semantics;
using ICSharpCode.Decompiler.TypeSystem; using ICSharpCode.Decompiler.TypeSystem;
using ICSharpCode.Decompiler.TypeSystem.Implementation; using ICSharpCode.Decompiler.TypeSystem.Implementation;
@ -3776,9 +3777,19 @@ namespace ICSharpCode.Decompiler.CSharp
protected internal override TranslatedExpression VisitAddressOf(AddressOf inst, TranslationContext context) protected internal override TranslatedExpression VisitAddressOf(AddressOf inst, TranslationContext context)
{ {
// HACK: this is only correct if the argument is an R-value; otherwise we're missing the copy to the temporary var classification = ILInlining.ClassifyExpression(inst.Value);
var value = Translate(inst.Value, inst.Type); var value = Translate(inst.Value, inst.Type);
value = value.ConvertTo(inst.Type, this); value = value.ConvertTo(inst.Type, this);
// ILAst AddressOf copies the value to a temporary, but when invoking a method in C#
// on a mutable lvalue, we would end up modifying the original lvalue, not just the copy.
// We solve this by introducing a "redundant" cast. Casts are classified as rvalue
// and ensure that the C# compiler will also create a copy.
if (classification == ExpressionClassification.MutableLValue && value.Expression is not CastExpression)
{
value = new CastExpression(ConvertType(inst.Type), value.Expression)
.WithoutILInstruction()
.WithRR(new ConversionResolveResult(inst.Type, value.ResolveResult, Conversion.IdentityConversion));
}
return new DirectionExpression(FieldDirection.Ref, value) return new DirectionExpression(FieldDirection.Ref, value)
.WithILInstruction(inst) .WithILInstruction(inst)
.WithRR(new ByReferenceResolveResult(value.ResolveResult, ReferenceKind.Ref)); .WithRR(new ByReferenceResolveResult(value.ResolveResult, ReferenceKind.Ref));

62
ICSharpCode.Decompiler/CSharp/Resolver/CastResolveResult.cs

@ -1,62 +0,0 @@
// Copyright (c) 2010-2013 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 ICSharpCode.Decompiler.Semantics;
using ICSharpCode.Decompiler.TypeSystem;
namespace ICSharpCode.Decompiler.CSharp.Resolver
{
/// <summary>
/// Represents an explicitly applied conversion (CastExpression or AsExpression)
/// (a result belonging to an AST node; not implicitly inserted 'between' nodes).
/// </summary>
class CastResolveResult : ConversionResolveResult
{
// The reason this class exists is that for code like this:
// int i = ...;
// long n = 0;
// n = n + (long)i;
// The resolver will produce (and process) an CastResolveResult for the cast,
// (with Conversion = implicit numeric conversion)
// and then pass it into CSharpResolver.ResolveBinaryOperator().
// That method normally wraps input arguments into another conversion
// (the implicit conversion applied by the operator).
// However, identity conversions do not cause the creation of ConversionResolveResult instances,
// so the OperatorResolveResult's argument will be the CastResolveResult
// of the cast.
// Without this class (and instead using ConversionResolveResult for both purposes),
// it would be hard for the conversion-processing code
// in the ResolveVisitor to distinguish the existing conversion from the CastExpression
// from an implicit conversion introduced by the binary operator.
// This would cause the conversion to be processed yet again.
// The following unit tests would fail without this class:
// * CastTests.ExplicitConversion_In_Assignment
// * FindReferencesTest.FindReferencesForOpImplicitInAssignment_ExplicitCast
// * CS0029InvalidConversionIssueTests.ExplicitConversionFromUnknownType
public CastResolveResult(ConversionResolveResult rr)
: base(rr.Type, rr.Input, rr.Conversion, rr.CheckForOverflow)
{
}
public CastResolveResult(IType targetType, ResolveResult input, Conversion conversion, bool checkForOverflow)
: base(targetType, input, conversion, checkForOverflow)
{
}
}
}

1
ICSharpCode.Decompiler/ICSharpCode.Decompiler.csproj

@ -275,7 +275,6 @@
<Compile Include="CSharp\Resolver\AliasNamespaceResolveResult.cs" /> <Compile Include="CSharp\Resolver\AliasNamespaceResolveResult.cs" />
<Compile Include="CSharp\Resolver\AliasTypeResolveResult.cs" /> <Compile Include="CSharp\Resolver\AliasTypeResolveResult.cs" />
<Compile Include="CSharp\Resolver\AwaitResolveResult.cs" /> <Compile Include="CSharp\Resolver\AwaitResolveResult.cs" />
<Compile Include="CSharp\Resolver\CastResolveResult.cs" />
<Compile Include="CSharp\Resolver\CSharpConversions.cs" /> <Compile Include="CSharp\Resolver\CSharpConversions.cs" />
<Compile Include="CSharp\Resolver\CSharpInvocationResolveResult.cs" /> <Compile Include="CSharp\Resolver\CSharpInvocationResolveResult.cs" />
<Compile Include="CSharp\Resolver\CSharpOperators.cs" /> <Compile Include="CSharp\Resolver\CSharpOperators.cs" />

25
ICSharpCode.Decompiler/IL/Transforms/ILInlining.cs

@ -240,7 +240,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
var loadInst = r.LoadInst; var loadInst = r.LoadInst;
if (loadInst.OpCode == OpCode.LdLoca) if (loadInst.OpCode == OpCode.LdLoca)
{ {
if (!IsGeneratedValueTypeTemporary((LdLoca)loadInst, v, inlinedExpression)) if (!IsGeneratedValueTypeTemporary((LdLoca)loadInst, v, inlinedExpression, options))
return false; return false;
} }
else else
@ -285,7 +285,7 @@ namespace ICSharpCode.Decompiler.IL.Transforms
/// </summary> /// </summary>
/// <param name="loadInst">The load instruction (a descendant within 'next')</param> /// <param name="loadInst">The load instruction (a descendant within 'next')</param>
/// <param name="v">The variable being inlined.</param> /// <param name="v">The variable being inlined.</param>
static bool IsGeneratedValueTypeTemporary(LdLoca loadInst, ILVariable v, ILInstruction inlinedExpression) static bool IsGeneratedValueTypeTemporary(LdLoca loadInst, ILVariable v, ILInstruction inlinedExpression, InliningOptions options)
{ {
Debug.Assert(loadInst.Variable == v); Debug.Assert(loadInst.Variable == v);
// Inlining a value type variable is allowed only if the resulting code will maintain the semantics // Inlining a value type variable is allowed only if the resulting code will maintain the semantics
@ -295,6 +295,13 @@ namespace ICSharpCode.Decompiler.IL.Transforms
// of the rvalue (e.g. M(ref (MyStruct)obj); is invalid). // of the rvalue (e.g. M(ref (MyStruct)obj); is invalid).
if (IsUsedAsThisPointerInCall(loadInst, out var method)) if (IsUsedAsThisPointerInCall(loadInst, out var method))
{ {
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)) switch (ClassifyExpression(inlinedExpression))
{ {
case ExpressionClassification.RValue: case ExpressionClassification.RValue:
@ -397,13 +404,6 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return inst != ldloca && inst.Parent is LdObj; return inst != ldloca && inst.Parent is LdObj;
} }
internal enum ExpressionClassification
{
RValue,
MutableLValue,
ReadonlyLValue,
}
/// <summary> /// <summary>
/// Gets whether the instruction, when converted into C#, turns into an l-value that can /// Gets whether the instruction, when converted into C#, turns into an l-value that can
/// be used to mutate a value-type. /// be used to mutate a value-type.
@ -828,4 +828,11 @@ namespace ICSharpCode.Decompiler.IL.Transforms
return CanMoveInto(arg, stmt, arg); return CanMoveInto(arg, stmt, arg);
} }
} }
internal enum ExpressionClassification
{
RValue,
MutableLValue,
ReadonlyLValue,
}
} }

Loading…
Cancel
Save