Browse Source

Fix #1454: Name collision after converting for-over-array to foreach.

pull/1464/head
Siegfried Pammer 6 years ago
parent
commit
b0cb976357
  1. 1
      ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj
  2. 6
      ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs
  3. 11
      ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs
  4. 49
      ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs

1
ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@ -72,6 +72,7 @@ @@ -72,6 +72,7 @@
<ItemGroup>
<Compile Include="DisassemblerPrettyTestRunner.cs" />
<Compile Include="TestCases\ILPretty\Issue1389.cs" />
<Compile Include="TestCases\ILPretty\Issue1454.cs" />
<Compile Include="TestCases\Pretty\Discards.cs" />
<Compile Include="TestCases\Pretty\MultidimensionalArray.cs" />
<Compile Include="Output\CSharpAmbienceTests.cs" />

6
ICSharpCode.Decompiler.Tests/ILPrettyTestRunner.cs

@ -142,6 +142,12 @@ namespace ICSharpCode.Decompiler.Tests @@ -142,6 +142,12 @@ namespace ICSharpCode.Decompiler.Tests
Run();
}
[Test]
public void Issue1454()
{
Run();
}
[Test]
public void SequenceOfNestedIfs()
{

11
ICSharpCode.Decompiler/CSharp/Transforms/DeclareVariables.cs

@ -101,6 +101,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -101,6 +101,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
public IdentifierExpression FirstUse;
public VariableToDeclare ReplacementDueToCollision;
public bool InvolvedInCollision;
public bool RemovedDueToCollision => ReplacementDueToCollision != null;
public VariableToDeclare(ILVariable variable, bool defaultInitialization, InsertionPoint insertionPoint, IdentifierExpression firstUse, int sourceOrder)
@ -158,6 +159,15 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -158,6 +159,15 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
return v.InsertionPoint.nextNode;
}
/// <summary>
/// Determines whether a variable was merged with other variables.
/// </summary>
public bool WasMerged(ILVariable variable)
{
VariableToDeclare v = variableDict[variable];
return v.InvolvedInCollision || v.RemovedDueToCollision;
}
public void ClearAnalysisResults()
{
variableDict.Clear();
@ -374,6 +384,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -374,6 +384,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
Debug.Assert(point1.level == point2.level);
if (point1.nextNode.Parent == point2.nextNode.Parent) {
// We found a collision!
v.InvolvedInCollision = true;
prev.ReplacementDueToCollision = v;
// Continue checking other entries in multiDict against the new position of `v`.
if (prev.SourceOrder < v.SourceOrder) {

49
ICSharpCode.Decompiler/CSharp/Transforms/PatternStatementTransform.cs

@ -251,6 +251,53 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -251,6 +251,53 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
}
};
bool VariableCanBeUsedAsForeachLocal(IL.ILVariable itemVar, Statement loop)
{
if (itemVar == null || !(itemVar.Kind == IL.VariableKind.Local || itemVar.Kind == IL.VariableKind.StackSlot)) {
// only locals/temporaries can be converted into foreach loop variable
return false;
}
var blockContainer = loop.Annotation<IL.BlockContainer>();
if (!itemVar.IsSingleDefinition) {
// foreach variable cannot be assigned to.
// As a special case, we accept taking the address for a method call,
// but only if the call is the only use, so that any mutation by the call
// cannot be observed.
if (!AddressUsedForSingleCall(itemVar, blockContainer)) {
return false;
}
}
if (itemVar.CaptureScope != null && itemVar.CaptureScope != blockContainer) {
// captured variables cannot be declared in the loop unless the loop is their capture scope
return false;
}
AstNode declPoint = declareVariables.GetDeclarationPoint(itemVar);
return declPoint.Ancestors.Contains(loop) && !declareVariables.WasMerged(itemVar);
}
static bool AddressUsedForSingleCall(IL.ILVariable v, IL.BlockContainer loop)
{
if (v.StoreCount == 1 && v.AddressCount == 1 && v.LoadCount == 0 && v.Type.IsReferenceType == false) {
if (v.AddressInstructions[0].Parent is IL.Call call
&& v.AddressInstructions[0].ChildIndex == 0
&& !call.Method.IsStatic) {
// used as this pointer for a method call
// this is OK iff the call is not within a nested loop
for (var node = call.Parent; node != null; node = node.Parent) {
if (node == loop)
return true;
else if (node is IL.BlockContainer)
break;
}
}
}
return false;
}
Statement TransformForeachOnArray(ForStatement forStatement)
{
if (!context.Settings.ForEachStatement) return null;
@ -262,7 +309,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms @@ -262,7 +309,7 @@ namespace ICSharpCode.Decompiler.CSharp.Transforms
var loopContainer = forStatement.Annotation<IL.BlockContainer>();
if (itemVariable == null || indexVariable == null || arrayVariable == null)
return null;
if (!itemVariable.IsSingleDefinition || (itemVariable.CaptureScope != null && itemVariable.CaptureScope != loopContainer))
if (!VariableCanBeUsedAsForeachLocal(itemVariable, forStatement))
return null;
if (indexVariable.StoreCount != 2 || indexVariable.LoadCount != 3 || indexVariable.AddressCount != 0)
return null;

Loading…
Cancel
Save