Browse Source

[nullables] Fix nullable transform being applied incorrectly.

pull/863/head
Daniel Grunwald 8 years ago
parent
commit
3c31e100ad
  1. 6
      ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs
  2. 2
      ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj
  3. 58
      ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs
  4. 68
      ICSharpCode.Decompiler.Tests/Util/BitSetTests.cs
  5. 55
      ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs
  6. 30
      ICSharpCode.Decompiler/Util/BitSet.cs

6
ICSharpCode.Decompiler.Tests/CorrectnessTestRunner.cs

@ -95,6 +95,12 @@ namespace ICSharpCode.Decompiler.Tests @@ -95,6 +95,12 @@ namespace ICSharpCode.Decompiler.Tests
RunCS(options: options);
}
[Test]
public void NullableTests([ValueSource("defaultOptions")] CompilerOptions options)
{
RunCS(options: options);
}
[Test]
public void Generics([ValueSource("defaultOptions")] CompilerOptions options)
{

2
ICSharpCode.Decompiler.Tests/ICSharpCode.Decompiler.Tests.csproj

@ -56,6 +56,7 @@ @@ -56,6 +56,7 @@
<Compile Include="Helpers\TypeSystemHelper.cs" />
<Compile Include="ILPrettyTestRunner.cs" />
<Compile Include="Stub.cs" />
<Compile Include="TestCases\Correctness\NullableTests.cs" />
<Compile Include="TestCases\Correctness\TrickyTypes.cs" />
<Compile Include="TestCases\ILPretty\Issue379.cs" />
<Compile Include="TestCases\Pretty\Async.cs" />
@ -97,6 +98,7 @@ @@ -97,6 +98,7 @@
<Compile Include="TestCases\Pretty\ShortCircuit.cs" />
<Compile Include="TestCases\Pretty\TypeAnalysisTests.cs" />
<Compile Include="TestTraceListener.cs" />
<Compile Include="Util\BitSetTests.cs" />
<Compile Include="Util\IntervalTests.cs" />
<Compile Include="Util\LongSetTests.cs" />
<Compile Include="CustomAttributes\CustomAttributeTests.cs" />

58
ICSharpCode.Decompiler.Tests/TestCases/Correctness/NullableTests.cs

@ -0,0 +1,58 @@ @@ -0,0 +1,58 @@
// Copyright (c) 2017 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;
namespace ICSharpCode.Decompiler.Tests.TestCases.Correctness
{
class NullableTests
{
static void Main()
{
Console.WriteLine("MayThrow:");
Console.WriteLine(MayThrow(10, 2, 3));
Console.WriteLine(MayThrow(10, 0, null));
Console.WriteLine("NotUsingAllInputs:");
Console.WriteLine(NotUsingAllInputs(5, 3));
Console.WriteLine(NotUsingAllInputs(5, null));
Console.WriteLine("UsingUntestedValue:");
Console.WriteLine(NotUsingAllInputs(5, 3));
Console.WriteLine(NotUsingAllInputs(5, null));
}
static int? MayThrow(int? a, int? b, int? c)
{
// cannot be lifted without changing the exception behavior
return a.HasValue & b.HasValue & c.HasValue ? a.GetValueOrDefault() / b.GetValueOrDefault() + c.GetValueOrDefault() : default(int?);
}
static int? NotUsingAllInputs(int? a, int? b)
{
// cannot be lifted because the value differs if b == null
return a.HasValue & b.HasValue ? a.GetValueOrDefault() + a.GetValueOrDefault() : default(int?);
}
static int? UsingUntestedValue(int? a, int? b)
{
// cannot be lifted because the value differs if b == null
return a.HasValue ? a.GetValueOrDefault() + b.GetValueOrDefault() : default(int?);
}
}
}

68
ICSharpCode.Decompiler.Tests/Util/BitSetTests.cs

@ -0,0 +1,68 @@ @@ -0,0 +1,68 @@
// Copyright (c) 2017 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 ICSharpCode.Decompiler.Util;
using NUnit.Framework;
namespace ICSharpCode.Decompiler.Tests.Util
{
[TestFixture]
public class BitSetTests
{
[Test]
public void SetRange()
{
var bitset = new BitSet(302);
bitset.Set(2, 300);
Assert.IsFalse(bitset[0]);
Assert.IsFalse(bitset[1]);
for (int i = 2; i < 300; ++i) {
Assert.IsTrue(bitset[i]);
}
Assert.IsFalse(bitset[301]);
}
[Test]
public void ClearRange()
{
var bitset = new BitSet(300);
bitset.Set(0, 300);
bitset.Clear(1, 299);
Assert.IsTrue(bitset[0]);
for (int i = 1; i < 299; ++i) {
Assert.IsFalse(bitset[i]);
}
Assert.IsTrue(bitset[299]);
}
[Test]
public void AllInRange()
{
var bitset = new BitSet(300);
bitset.Set(1, 299);
Assert.IsTrue(bitset.All(1, 299));
Assert.IsTrue(bitset.All(10, 290));
Assert.IsTrue(bitset.All(100, 200));
Assert.IsFalse(bitset.All(0, 200));
Assert.IsFalse(bitset.All(0, 1));
Assert.IsFalse(bitset.All(1, 300));
bitset[200] = false;
Assert.IsFalse(bitset.All(1, 299));
}
}
}

55
ICSharpCode.Decompiler/IL/Transforms/NullableLiftingTransform.cs

@ -20,6 +20,7 @@ using System; @@ -20,6 +20,7 @@ using System;
using System.Collections.Generic;
using System.Diagnostics;
using ICSharpCode.Decompiler.TypeSystem;
using ICSharpCode.Decompiler.Util;
namespace ICSharpCode.Decompiler.IL.Transforms
{
@ -153,7 +154,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -153,7 +154,12 @@ namespace ICSharpCode.Decompiler.IL.Transforms
};
} else {
context.Step("NullableLiftingTransform.DoLift", ifInst);
lifted = DoLift(exprToLift);
BitSet bits;
(lifted, bits) = DoLift(exprToLift);
if (lifted != null && !bits.All(0, nullableVars.Count)) {
// don't lift if a nullableVar doesn't contribute to the result
lifted = null;
}
}
if (lifted != null) {
Debug.Assert(lifted is ILiftableInstruction liftable && liftable.IsLifted
@ -164,22 +170,39 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -164,22 +170,39 @@ namespace ICSharpCode.Decompiler.IL.Transforms
// Lifts the specified instruction.
// Creates a new lifted instruction without modifying the input instruction.
// If lifting fails, returns null.
ILInstruction DoLift(ILInstruction inst)
// Returns (new lifted instruction, bitset of nullableVars that will cause the expression to evaluate to null).
// If lifting fails, returns (null, null).
(ILInstruction, BitSet) DoLift(ILInstruction inst)
{
if (MatchGetValueOrDefault(inst, out ILVariable inputVar) && nullableVars.Contains(inputVar)) {
if (MatchGetValueOrDefault(inst, out ILVariable inputVar)) {
// n.GetValueOrDefault() lifted => n.
return new LdLoc(inputVar) { ILRange = inst.ILRange };
BitSet foundIndices = new BitSet(nullableVars.Count);
for (int i = 0; i < nullableVars.Count; i++) {
if (nullableVars[i] == inputVar) {
foundIndices[i] = true;
}
}
if (foundIndices.Any())
return (new LdLoc(inputVar) { ILRange = inst.ILRange }, foundIndices);
else
return (null, null);
} else if (inst is Conv conv) {
var arg = DoLift(conv.Argument);
var (arg, bits) = DoLift(conv.Argument);
if (arg != null) {
return new Conv(arg, conv.InputType, conv.InputSign, conv.TargetType, conv.CheckForOverflow, isLifted: true) {
if (conv.HasFlag(InstructionFlags.MayThrow) && !bits.All(0, nullableVars.Count)) {
// Cannot execute potentially-throwing instruction unless all
// the nullableVars are arguments to the instruction
// (thus causing it not to throw when any of them is null).
return (null, null);
}
var newInst = new Conv(arg, conv.InputType, conv.InputSign, conv.TargetType, conv.CheckForOverflow, isLifted: true) {
ILRange = conv.ILRange
};
return (newInst, bits);
}
} else if (inst is BinaryNumericInstruction binary) {
var left = DoLift(binary.Left);
var right = DoLift(binary.Right);
var (left, leftBits) = DoLift(binary.Left);
var (right, rightBits) = DoLift(binary.Right);
if (left != null && right == null && SemanticHelper.IsPure(binary.Right.Flags)) {
// Embed non-nullable pure expression in lifted expression.
right = binary.Right.Clone();
@ -189,7 +212,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -189,7 +212,16 @@ namespace ICSharpCode.Decompiler.IL.Transforms
left = binary.Left.Clone();
}
if (left != null && right != null) {
return new BinaryNumericInstruction(
var bits = leftBits ?? rightBits;
if (rightBits != null)
bits.UnionWith(rightBits);
if (binary.HasFlag(InstructionFlags.MayThrow) && !bits.All(0, nullableVars.Count)) {
// Cannot execute potentially-throwing instruction unless all
// the nullableVars are arguments to the instruction
// (thus causing it not to throw when any of them is null).
return (null, null);
}
var newInst = new BinaryNumericInstruction(
binary.Operator, left, right,
binary.LeftInputType, binary.RightInputType,
binary.CheckForOverflow, binary.Sign,
@ -197,9 +229,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms @@ -197,9 +229,10 @@ namespace ICSharpCode.Decompiler.IL.Transforms
) {
ILRange = binary.ILRange
};
return (newInst, bits);
}
}
return null;
return (null, null);
}
#endregion

30
ICSharpCode.Decompiler/Util/BitSet.cs

@ -80,7 +80,33 @@ namespace ICSharpCode.Decompiler.Util @@ -80,7 +80,33 @@ namespace ICSharpCode.Decompiler.Util
}
return false;
}
/// <summary>
/// Gets whether all bits in the specified range are set.
/// </summary>
public bool All(int startIndex, int endIndex)
{
Debug.Assert(startIndex <= endIndex);
if (startIndex >= endIndex) {
return true;
}
int startWordIndex = WordIndex(startIndex);
int endWordIndex = WordIndex(endIndex - 1);
ulong startMask = Mask << startIndex;
ulong endMask = Mask >> -endIndex; // same as (Mask >> (64 - (endIndex % 64)))
if (startWordIndex == endWordIndex) {
return (words[startWordIndex] & (startMask & endMask)) == (startMask & endMask);
} else {
if ((words[startWordIndex] & startMask) != startMask)
return false;
for (int i = startWordIndex + 1; i < endWordIndex; i++) {
if (words[i] != ulong.MaxValue)
return false;
}
return (words[endWordIndex] & endMask) == endMask;
}
}
/// <summary>
/// Gets whether both bitsets have the same content.
/// </summary>
@ -174,7 +200,7 @@ namespace ICSharpCode.Decompiler.Util @@ -174,7 +200,7 @@ namespace ICSharpCode.Decompiler.Util
} else {
words[startWordIndex] |= startMask;
for (int i = startWordIndex + 1; i < endWordIndex; i++) {
words[i] = 0;
words[i] = ulong.MaxValue;
}
words[endWordIndex] |= endMask;
}

Loading…
Cancel
Save