Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Patterns: Allow non-negative and full integer sets to merge (intersect or union) #71968

Merged
merged 20 commits into from
Mar 6, 2024
Merged

Patterns: Allow non-negative and full integer sets to merge (intersect or union) #71968

Show file tree
Hide file tree
Changes from 12 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
45bd785
Patterns: Expand non-negative sets when performing certain operations
jcouv Feb 6, 2024
9f74cdb
Add test
jcouv Feb 6, 2024
af16224
Fix test
jcouv Feb 6, 2024
1bb85ae
Add tests and experiment with shrinking
jcouv Feb 22, 2024
e6da7f4
Stick with expansion
jcouv Feb 22, 2024
e0597e4
Use instances for INumericTC implementations
jcouv Feb 23, 2024
25de777
Avoid allocations
jcouv Feb 23, 2024
ac2f593
Use a struct for NumericValueSetFactory
jcouv Feb 23, 2024
b8eacd1
Tweak
jcouv Feb 23, 2024
b725762
Typo
jcouv Feb 23, 2024
31e9af7
Renames
jcouv Feb 23, 2024
af60453
private
jcouv Feb 23, 2024
d617b80
Remove TTC type parameters
jcouv Feb 23, 2024
aa29183
restore NonNegativeIntValueSetFactory
jcouv Feb 23, 2024
355aa2c
Add assertions
jcouv Feb 23, 2024
78678e6
Address feedback
jcouv Feb 23, 2024
043cebb
Restore cached interface in NonNegativeIntValueSetFactory
jcouv Feb 23, 2024
768d63f
Typo
jcouv Feb 26, 2024
58ce1ee
Re-use MinValue
jcouv Feb 26, 2024
8bb84f9
Update src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.Numeri…
jcouv Mar 6, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.ByteTC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ namespace Microsoft.CodeAnalysis.CSharp

internal static partial class ValueSetFactory
{
private struct ByteTC : INumericTC<byte>
private class ByteTC : INumericTC<byte>
{
public static readonly ByteTC Instance = new ByteTC();

byte INumericTC<byte>.MinValue => byte.MinValue;

byte INumericTC<byte>.MaxValue => byte.MaxValue;
Expand Down
5 changes: 3 additions & 2 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.CharTC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@

using System;
using System.Diagnostics;
using System.Globalization;

namespace Microsoft.CodeAnalysis.CSharp
{
using static BinaryOperatorKind;

internal static partial class ValueSetFactory
{
private struct CharTC : INumericTC<char>
private class CharTC : INumericTC<char>
{
public static readonly CharTC Instance = new CharTC();

char INumericTC<char>.MinValue => char.MinValue;

char INumericTC<char>.MaxValue => char.MaxValue;
Expand Down
6 changes: 4 additions & 2 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.DecimalTC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@ namespace Microsoft.CodeAnalysis.CSharp

internal static partial class ValueSetFactory
{
private struct DecimalTC : INumericTC<decimal>
private class DecimalTC : INumericTC<decimal>
{
public static readonly DecimalTC Instance = new DecimalTC();

// These are the smallest nonzero normal mantissa value (in three parts) below which you could use a higher scale.
// This is the 96-bit representation of ((2^96)-1) / 10;
private const uint transitionLow = 0x99999999;
Expand Down Expand Up @@ -112,7 +114,7 @@ decimal INumericTC<decimal>.Prev(decimal value)

public decimal Random(Random random)
{
INumericTC<uint> uinttc = default(UIntTC);
INumericTC<uint> uinttc = UIntTC.Instance;
return new DecimalRep(
low: uinttc.Random(random),
mid: uinttc.Random(random),
Expand Down
10 changes: 5 additions & 5 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.DecimalValueSetFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ private sealed class DecimalValueSetFactory : IValueSetFactory<decimal>, IValueS
{
public static readonly DecimalValueSetFactory Instance = new DecimalValueSetFactory();

private readonly IValueSetFactory<decimal> _underlying = NumericValueSetFactory<decimal, DecimalTC>.Instance;
private readonly IValueSetFactory<decimal> _underlying = new NumericValueSetFactory<decimal, DecimalTC>(DecimalTC.Instance);

IValueSet IValueSetFactory.AllValues => NumericValueSet<decimal, DecimalTC>.AllValues;
IValueSet IValueSetFactory.AllValues => NumericValueSet<decimal, DecimalTC>.AllValues(DecimalTC.Instance);

IValueSet IValueSetFactory.NoValues => NumericValueSet<decimal, DecimalTC>.NoValues;
IValueSet IValueSetFactory.NoValues => NumericValueSet<decimal, DecimalTC>.NoValues(DecimalTC.Instance);

public IValueSet<decimal> Related(BinaryOperatorKind relation, decimal value) => _underlying.Related(relation, DecimalTC.Normalize(value));

IValueSet IValueSetFactory.Random(int expectedSize, Random random) => _underlying.Random(expectedSize, random);

ConstantValue IValueSetFactory.RandomValue(Random random) => ConstantValue.Create(default(DecimalTC).Random(random));
ConstantValue IValueSetFactory.RandomValue(Random random) => ConstantValue.Create(DecimalTC.Instance.Random(random));

IValueSet IValueSetFactory.Related(BinaryOperatorKind relation, ConstantValue value) =>
value.IsBad ? NumericValueSet<decimal, DecimalTC>.AllValues : Related(relation, default(DecimalTC).FromConstantValue(value));
value.IsBad ? NumericValueSet<decimal, DecimalTC>.AllValues(DecimalTC.Instance) : Related(relation, DecimalTC.Instance.FromConstantValue(value));

bool IValueSetFactory.Related(BinaryOperatorKind relation, ConstantValue left, ConstantValue right) => _underlying.Related(relation, left, right);
}
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.DoubleTC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ namespace Microsoft.CodeAnalysis.CSharp

internal static partial class ValueSetFactory
{
private struct DoubleTC : FloatingTC<double>, INumericTC<double>
private class DoubleTC : FloatingTC<double>, INumericTC<double>
{
public static readonly DoubleTC Instance = new DoubleTC();

double INumericTC<double>.MinValue => double.NegativeInfinity;

double INumericTC<double>.MaxValue => double.PositiveInfinity;
Expand Down
55 changes: 29 additions & 26 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.FloatingValueSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,32 +18,33 @@ internal static partial class ValueSetFactory
/// </summary>
/// <typeparam name="TFloating">A floating-point type.</typeparam>
/// <typeparam name="TFloatingTC">A typeclass supporting that floating-point type.</typeparam>
private sealed class FloatingValueSet<TFloating, TFloatingTC> : IValueSet<TFloating> where TFloatingTC : struct, FloatingTC<TFloating>
private sealed class FloatingValueSet<TFloating, TFloatingTC> : IValueSet<TFloating> where TFloatingTC : class, FloatingTC<TFloating>
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFloatingTC

I think we should be able to get rid of this type parameter #Closed

{
private readonly IValueSet<TFloating> _numbers;
private readonly bool _hasNaN;
private readonly TFloatingTC _tc;

private FloatingValueSet(IValueSet<TFloating> numbers, bool hasNaN)
private FloatingValueSet(IValueSet<TFloating> numbers, bool hasNaN, TFloatingTC tc)
{
RoslynDebug.Assert(numbers is NumericValueSet<TFloating, TFloatingTC>);
(_numbers, _hasNaN) = (numbers, hasNaN);
(_numbers, _hasNaN, _tc) = (numbers, hasNaN, tc);
}

internal static readonly IValueSet<TFloating> AllValues = new FloatingValueSet<TFloating, TFloatingTC>(
numbers: NumericValueSet<TFloating, TFloatingTC>.AllValues, hasNaN: true);
internal static IValueSet<TFloating> AllValues(TFloatingTC tc) => new FloatingValueSet<TFloating, TFloatingTC>(
numbers: NumericValueSet<TFloating, TFloatingTC>.AllValues(tc), hasNaN: true, tc);

internal static readonly IValueSet<TFloating> NoValues = new FloatingValueSet<TFloating, TFloatingTC>(
numbers: NumericValueSet<TFloating, TFloatingTC>.NoValues, hasNaN: false);
internal static IValueSet<TFloating> NoValues(TFloatingTC tc) => new FloatingValueSet<TFloating, TFloatingTC>(
numbers: NumericValueSet<TFloating, TFloatingTC>.NoValues(tc), hasNaN: false, tc);

internal static IValueSet<TFloating> Random(int expectedSize, Random random)
internal static IValueSet<TFloating> Random(int expectedSize, Random random, TFloatingTC tc)
{
bool hasNan = random.NextDouble() < 0.5;
if (hasNan)
expectedSize--;
if (expectedSize < 1)
expectedSize = 2;
return new FloatingValueSet<TFloating, TFloatingTC>(
numbers: (IValueSet<TFloating>)NumericValueSetFactory<TFloating, TFloatingTC>.Instance.Random(expectedSize, random), hasNaN: hasNan);
numbers: (IValueSet<TFloating>)new NumericValueSetFactory<TFloating, TFloatingTC>(tc).Random(expectedSize, random), hasNaN: hasNan, tc);
}

public bool IsEmpty => !_hasNaN && _numbers.IsEmpty;
Expand All @@ -63,14 +64,12 @@ ConstantValue IValueSet.Sample
}

Debug.Assert(_hasNaN);
var tc = default(TFloatingTC);
return tc.ToConstantValue(tc.NaN);
return _tc.ToConstantValue(_tc.NaN);
}
}

public static IValueSet<TFloating> Related(BinaryOperatorKind relation, TFloating value)
public static IValueSet<TFloating> Related(BinaryOperatorKind relation, TFloating value, TFloatingTC tc)
{
TFloatingTC tc = default;
if (tc.Related(Equal, tc.NaN, value))
{
switch (relation)
Expand All @@ -80,18 +79,20 @@ public static IValueSet<TFloating> Related(BinaryOperatorKind relation, TFloatin
case BinaryOperatorKind.GreaterThanOrEqual:
return new FloatingValueSet<TFloating, TFloatingTC>(
hasNaN: true,
numbers: NumericValueSet<TFloating, TFloatingTC>.NoValues
numbers: NumericValueSet<TFloating, TFloatingTC>.NoValues(tc),
tc: tc
);
case BinaryOperatorKind.LessThan:
case BinaryOperatorKind.GreaterThan:
return NoValues;
return NoValues(tc);
default:
throw ExceptionUtilities.UnexpectedValue(relation);
}
}
return new FloatingValueSet<TFloating, TFloatingTC>(
numbers: NumericValueSetFactory<TFloating, TFloatingTC>.Instance.Related(relation, value),
hasNaN: false
numbers: new NumericValueSetFactory<TFloating, TFloatingTC>(tc).Related(relation, value),
hasNaN: false,
tc: tc
);
}

Expand All @@ -100,9 +101,11 @@ public IValueSet<TFloating> Intersect(IValueSet<TFloating> o)
if (this == o)
return this;
var other = (FloatingValueSet<TFloating, TFloatingTC>)o;

return new FloatingValueSet<TFloating, TFloatingTC>(
numbers: this._numbers.Intersect(other._numbers),
hasNaN: this._hasNaN & other._hasNaN);
hasNaN: this._hasNaN & other._hasNaN,
_tc);
}

IValueSet IValueSet.Intersect(IValueSet other) => this.Intersect((IValueSet<TFloating>)other);
Expand All @@ -114,7 +117,8 @@ public IValueSet<TFloating> Union(IValueSet<TFloating> o)
var other = (FloatingValueSet<TFloating, TFloatingTC>)o;
return new FloatingValueSet<TFloating, TFloatingTC>(
numbers: this._numbers.Union(other._numbers),
hasNaN: this._hasNaN | other._hasNaN);
hasNaN: this._hasNaN | other._hasNaN,
_tc);
}

IValueSet IValueSet.Union(IValueSet other) => this.Union((IValueSet<TFloating>)other);
Expand All @@ -123,29 +127,28 @@ public IValueSet<TFloating> Complement()
{
return new FloatingValueSet<TFloating, TFloatingTC>(
numbers: this._numbers.Complement(),
hasNaN: !this._hasNaN);
hasNaN: !this._hasNaN,
_tc);
}

IValueSet IValueSet.Complement() => this.Complement();

bool IValueSet.Any(BinaryOperatorKind relation, ConstantValue value) =>
value.IsBad || this.Any(relation, default(TFloatingTC).FromConstantValue(value));
value.IsBad || this.Any(relation, _tc.FromConstantValue(value));

public bool Any(BinaryOperatorKind relation, TFloating value)
{
TFloatingTC tc = default;
return
_hasNaN && tc.Related(relation, tc.NaN, value) ||
_hasNaN && _tc.Related(relation, _tc.NaN, value) ||
_numbers.Any(relation, value);
}

bool IValueSet.All(BinaryOperatorKind relation, ConstantValue value) => !value.IsBad && All(relation, default(TFloatingTC).FromConstantValue(value));
bool IValueSet.All(BinaryOperatorKind relation, ConstantValue value) => !value.IsBad && All(relation, _tc.FromConstantValue(value));

public bool All(BinaryOperatorKind relation, TFloating value)
{
TFloatingTC tc = default;
return
(!_hasNaN || tc.Related(relation, tc.NaN, value)) &&
(!_hasNaN || _tc.Related(relation, _tc.NaN, value)) &&
_numbers.All(relation, value);
}

Expand Down
27 changes: 15 additions & 12 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.FloatingValueSetFactory.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,35 +8,38 @@ namespace Microsoft.CodeAnalysis.CSharp
{
internal static partial class ValueSetFactory
{
private sealed class FloatingValueSetFactory<TFloating, TFloatingTC> : IValueSetFactory<TFloating> where TFloatingTC : struct, FloatingTC<TFloating>
private sealed class FloatingValueSetFactory<TFloating, TFloatingTC> : IValueSetFactory<TFloating> where TFloatingTC : class, FloatingTC<TFloating>
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TFloatingTC

I think we should be able to get rid of this type parameter #Closed

{
public static readonly FloatingValueSetFactory<TFloating, TFloatingTC> Instance = new FloatingValueSetFactory<TFloating, TFloatingTC>();
private readonly TFloatingTC _tc;

private FloatingValueSetFactory() { }
public FloatingValueSetFactory(TFloatingTC tc)
{
_tc = tc;
}

IValueSet IValueSetFactory.AllValues => FloatingValueSet<TFloating, TFloatingTC>.AllValues;
IValueSet IValueSetFactory.AllValues => FloatingValueSet<TFloating, TFloatingTC>.AllValues(_tc);

IValueSet IValueSetFactory.NoValues => FloatingValueSet<TFloating, TFloatingTC>.NoValues;
IValueSet IValueSetFactory.NoValues => FloatingValueSet<TFloating, TFloatingTC>.NoValues(_tc);

public IValueSet<TFloating> Related(BinaryOperatorKind relation, TFloating value) =>
FloatingValueSet<TFloating, TFloatingTC>.Related(relation, value);
FloatingValueSet<TFloating, TFloatingTC>.Related(relation, value, _tc);

IValueSet IValueSetFactory.Random(int expectedSize, Random random) =>
FloatingValueSet<TFloating, TFloatingTC>.Random(expectedSize, random);
FloatingValueSet<TFloating, TFloatingTC>.Random(expectedSize, random, _tc);

ConstantValue IValueSetFactory.RandomValue(Random random)
{
TFloatingTC tc = default;
return tc.ToConstantValue(tc.Random(random));
return _tc.ToConstantValue(_tc.Random(random));
}

IValueSet IValueSetFactory.Related(BinaryOperatorKind relation, ConstantValue value) =>
value.IsBad ? FloatingValueSet<TFloating, TFloatingTC>.AllValues : FloatingValueSet<TFloating, TFloatingTC>.Related(relation, default(TFloatingTC).FromConstantValue(value));
value.IsBad
? FloatingValueSet<TFloating, TFloatingTC>.AllValues(_tc)
: FloatingValueSet<TFloating, TFloatingTC>.Related(relation, _tc.FromConstantValue(value), _tc);

bool IValueSetFactory.Related(BinaryOperatorKind relation, ConstantValue left, ConstantValue right)
{
TFloatingTC tc = default;
return tc.Related(relation, tc.FromConstantValue(left), tc.FromConstantValue(right));
return _tc.Related(relation, _tc.FromConstantValue(left), _tc.FromConstantValue(right));
}
}
}
Expand Down
27 changes: 23 additions & 4 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.IntTC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,21 @@ namespace Microsoft.CodeAnalysis.CSharp

internal static partial class ValueSetFactory
{
private struct IntTC : INumericTC<int>
private class IntTC : INumericTC<int>
{
int INumericTC<int>.MinValue => int.MinValue;
// Note: whenever we intersect or union two sets of IntTCs,
// we just keep the nonNegative flag of the set we're merging into.
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 23, 2024 •
edited by jcouv
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

// we just keep the nonNegative flag of the set we're merging into.

If we are uncomfortable relying on this behavior, we could add Intersect/Union APIs to INumericTC and implement them for this type by preferring non-negative flavor among intersected, and preferring full-set among union-ed. NumericValueSet.Intersect/Union will call the same named APIs to get the resulting INumericTC to use. #WontFix

public bool nonNegative;
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

public bool nonNegative;

I really like the fact that we no longer have to use different types to represent this difference in behavior #Closed


private IntTC(bool nonNegative)
{
this.nonNegative = nonNegative;
}

public static readonly IntTC DefaultInstance = new IntTC(nonNegative: false);
public static readonly IntTC NonNegativeInstance = new IntTC(nonNegative: true);

int INumericTC<int>.MinValue => nonNegative ? 0 : int.MinValue;

int INumericTC<int>.MaxValue => int.MaxValue;

Expand Down Expand Up @@ -46,7 +58,7 @@ int INumericTC<int>.Next(int value)

int INumericTC<int>.Prev(int value)
{
Debug.Assert(value != int.MinValue);
Debug.Assert(value != (nonNegative ? 0 : int.MinValue));
Copy link
Contributor

@AlekseyTs AlekseyTs Feb 23, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nonNegative ? 0 : int.MinValue)

MinValue? #Closed

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinValue is a private interface implementation

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MinValue is a private interface implementation

It doesn't have to be private. And we can avoid code duplication.

return value - 1;
}

Expand All @@ -58,7 +70,14 @@ int INumericTC<int>.Prev(int value)

public int Random(Random random)
{
return (random.Next() << 10) ^ random.Next();
if (nonNegative)
{
return Math.Abs((random.Next() << 10) ^ random.Next());
}
else
{
return (random.Next() << 10) ^ random.Next();
}
}
}
}
Expand Down
4 changes: 3 additions & 1 deletion src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.LongTC.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,10 @@ namespace Microsoft.CodeAnalysis.CSharp

internal static partial class ValueSetFactory
{
private struct LongTC : INumericTC<long>
private class LongTC : INumericTC<long>
{
public static readonly LongTC Instance = new LongTC();

long INumericTC<long>.MinValue => long.MinValue;

long INumericTC<long>.MaxValue => long.MaxValue;
Expand Down
4 changes: 2 additions & 2 deletions src/Compilers/CSharp/Portable/Utilities/ValueSetFactory.NintValueSet.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,9 @@ internal static partial class ValueSetFactory
{
private sealed class NintValueSet : IValueSet<int>, IValueSet
{
public static readonly NintValueSet AllValues = new NintValueSet(hasSmall: true, values: NumericValueSet<int, IntTC>.AllValues, hasLarge: true);
public static readonly NintValueSet AllValues = new NintValueSet(hasSmall: true, values: NumericValueSet<int, IntTC>.AllValues(IntTC.DefaultInstance), hasLarge: true);

public static readonly NintValueSet NoValues = new NintValueSet(hasSmall: false, values: NumericValueSet<int, IntTC>.NoValues, hasLarge: false);
public static readonly NintValueSet NoValues = new NintValueSet(hasSmall: false, values: NumericValueSet<int, IntTC>.NoValues(IntTC.DefaultInstance), hasLarge: false);

private readonly IValueSet<int> _values;

Expand Down
Loading
Loading