Skip to content
This repository has been archived by the owner on Jan 23, 2023. It is now read-only.
/ corefx Public archive

Issue #24343 Vector Ctor using Span #26499

Merged
merged 1 commit into from
Mar 9, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
3 changes: 3 additions & 0 deletions src/System.Numerics.Vectors/ref/System.Numerics.Vectors.cs
Original file line number Diff line number Diff line change
Expand Up @@ -301,6 +301,9 @@ public partial struct Vector<T> : System.IEquatable<System.Numerics.Vector<T>>,
public Vector(T value) { throw null; }
public Vector(T[] values) { throw null; }
public Vector(T[] values, int index) { throw null; }
#if netcoreapp
Copy link
Member

Choose a reason for hiding this comment

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

Typically we put these into separate .netcoreapp.cs files instead of putting #if in the code. See https://github.com/dotnet/corefx/blob/master/src/System.Net.Sockets/ref/System.Net.Sockets.netcoreapp.cs for an example.

public Vector(Span<T> values) { throw null; }
#endif
public static int Count { get { throw null; } }
public T this[int index] { get { throw null; } }
public static System.Numerics.Vector<T> One { get { throw null; } }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,10 @@
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.props))\dir.props" />
<PropertyGroup>
<ProjectGuid>{650277B5-9423-4ACE-BB54-2659995B21C7}</ProjectGuid>
<IsPartialFacadeAssembly Condition="'$(TargetGroup)' == 'netfx' OR '$(TargetGroup)' == 'net46'">true</IsPartialFacadeAssembly>
<IsTargetingNetFx Condition="'$(TargetGroup)'=='netfx' OR '$(TargetGroup)'=='net46'">true</IsTargetingNetFx>
Copy link
Member

Choose a reason for hiding this comment

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

Do we need this condition?

Can we just collapse this into the IsPartialFacadeAssembly and use that?

<IsPartialFacadeAssembly Condition="'$(TargetGroup)'=='netfx' OR '$(TargetGroup)'=='net46'">true</IsPartialFacadeAssembly>

Copy link
Author

Choose a reason for hiding this comment

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

IsTargetingNetFx is also used at line 28 below. Hence made the changes to IsPartialFacadeAssembly to consolidate the conditions in one place.

Copy link
Member

Choose a reason for hiding this comment

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

These properties mirror what is in src\System.Numerics.Vectors.csproj

<IsTargetingNetFx Condition="'$(TargetGroup)'=='netfx' OR '$(TargetGroup)'=='net46'">true</IsTargetingNetFx>
<IsTargetingNetCoreApp Condition="'$(TargetGroup)'=='netcoreapp' OR '$(TargetGroup)'=='uap' OR '$(TargetGroup)'=='uapaot'">true</IsTargetingNetCoreApp>
<IsPartialFacadeAssembly Condition="'$(IsTargetingNetFx)'=='true' OR '$(IsTargetingNetCoreApp)'=='true'">true</IsPartialFacadeAssembly>
<PackageTargetFramework Condition="'$(TargetGroup)' == 'netstandard1.0'">netstandard1.0;portable-net45+win8+wp8+wpa81</PackageTargetFramework>

I think it is clearer to define them this way, so you can write code later that says:

"If targeting netfx, set this or that"

Instead of code that says:

"If is partial facade assembly, set this or that"

Then you need to constantly look up and say "When is this a partial facade?".

Copy link
Member

Choose a reason for hiding this comment

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

I think it is clearer to define them this way

Good point. I am convinced.

<IsTargetingNetCoreApp Condition="'$(TargetGroup)'=='netcoreapp' OR '$(TargetGroup)'=='uap'">true</IsTargetingNetCoreApp>
<IsPartialFacadeAssembly Condition="'$(IsTargetingNetFx)'=='true'">true</IsPartialFacadeAssembly>
<DefineConstants Condition="'$(IsTargetingNetCoreApp)'=='true'">$(DefineConstants);netcoreapp</DefineConstants>
</PropertyGroup>
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'net46-Debug|AnyCPU'" />
<PropertyGroup Condition="'$(Configuration)|$(Platform)' == 'net46-Release|AnyCPU'" />
Expand All @@ -22,14 +25,14 @@
<ItemGroup>
<Compile Include="System.Numerics.Vectors.cs" />
</ItemGroup>
<ItemGroup Condition="'$(TargetGroup)' == 'netfx' OR '$(TargetGroup)' == 'net46'">
<ItemGroup Condition="'$(IsTargetingNetFx)'=='true'">
<Reference Include="mscorlib" />
<Reference Include="System.Numerics" />
</ItemGroup>
<ItemGroup Condition="'$(TargetGroup)' == 'netstandard1.0'">
<Reference Include="System.Runtime" />
</ItemGroup>
<ItemGroup Condition="'$(TargetGroup)' == 'netcoreapp' OR '$(TargetGroup)' == 'uap' OR '$(TargetGroup)' == 'uapaot'">
<ItemGroup Condition="'$(IsTargetingNetCoreApp)'=='true'">
<ProjectReference Include="..\..\System.Runtime\ref\System.Runtime.csproj" />
</ItemGroup>
<Import Project="$([MSBuild]::GetDirectoryNameOfFileAbove($(MSBuildThisFileDirectory), dir.targets))\dir.targets" />
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
MembersMustExist : Member 'System.Numerics.Vector<T>..ctor(System.Span<T>)' does not exist in the implementation but it does exist in the contract.
Copy link
Member

Choose a reason for hiding this comment

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

Do we still need to add this exception for uapaot?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, as of a few hours ago this was needed. See #26499 (comment)

Copy link
Author

Choose a reason for hiding this comment

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

I added this as I was getting build errors. further discussion here.

8 changes: 4 additions & 4 deletions src/System.Numerics.Vectors/tests/GenericVectorTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ namespace System.Numerics.Tests
/// <summary>
/// Vector{T} tests that use random number generation and a unified generic test structure
/// </summary>
public class GenericVectorTests
public partial class GenericVectorTests
{
// Static constructor in top-level class\
static System.Numerics.Vector<float> dummy;
Expand Down Expand Up @@ -2699,11 +2699,11 @@ private static void ValidateVector<T>(Vector<T> vector, Action<int, T> indexVali
}
}

internal static T[] GenerateRandomValuesForVector<T>() where T : struct
internal static T[] GenerateRandomValuesForVector<T>(int? numValues = null) where T : struct
{
int minValue = GetMinValue<T>();
int maxValue = GetMaxValue<T>();
return Util.GenerateRandomValues<T>(Vector<T>.Count, minValue, maxValue);
return Util.GenerateRandomValues<T>(numValues ?? Vector<T>.Count, minValue, maxValue);
}

internal static int GetMinValue<T>() where T : struct
Expand Down Expand Up @@ -2776,4 +2776,4 @@ internal static T GetValueWithAllOnesSet<T>() where T : struct
}
#endregion
}
}
}
226 changes: 226 additions & 0 deletions src/System.Numerics.Vectors/tests/GenericVectorTests.netcoreapp.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,226 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.
// See the LICENSE file in the project root for more information.

using System;
using System.Globalization;
using System.Linq;
using System.Reflection;
using Xunit;
using Xunit.Sdk;

namespace System.Numerics.Tests
{
/// <summary>
/// Vector{T} tests that use random number generation and a unified generic test structure
/// </summary>
public partial class GenericVectorTests
{
#region Constructor Tests

#region Tests for Span based constructor
[Fact]
public void ConstructorWithSpanByte() => TestConstructorWithSpan<Byte>();
[Fact]
public void ConstructorWithSpanSByte() => TestConstructorWithSpan<SByte>();
[Fact]
public void ConstructorWithSpanUInt16() => TestConstructorWithSpan<UInt16>();
[Fact]
public void ConstructorWithSpanInt16() => TestConstructorWithSpan<Int16>();
[Fact]
public void ConstructorWithSpanUInt32() => TestConstructorWithSpan<UInt32>();
[Fact]
public void ConstructorWithSpanInt32() => TestConstructorWithSpan<Int32>();
[Fact]
public void ConstructorWithSpanUInt64() => TestConstructorWithSpan<UInt64>();
[Fact]
public void ConstructorWithSpanInt64() => TestConstructorWithSpan<Int64>();
[Fact]
public void ConstructorWithSpanSingle() => TestConstructorWithSpan<Single>();
[Fact]
public void ConstructorWithSpanDouble() => TestConstructorWithSpan<Double>();

private void TestConstructorWithSpan<T>() where T : struct
{
T[] values = GenerateRandomValuesForVector<T>().ToArray();
var valueSpan = new Span<T>(values);

var vector = new Vector<T>(valueSpan);
ValidateVector(vector,
(index, val) =>
{
Assert.Equal(values[index], val);
});
}

[Fact]
public void SpanBasedConstructorWithLessElements_Byte() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<Byte>());
[Fact]
public void SpanBasedConstructorWithLessElements_SByte() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<SByte>());
[Fact]
public void SpanBasedConstructorWithLessElements_UInt16() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<UInt16>());
[Fact]
public void SpanBasedConstructorWithLessElements_Int16() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<Int16>());
[Fact]
public void SpanBasedConstructorWithLessElements_UInt32() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<UInt32>());
[Fact]
public void SpanBasedConstructorWithLessElements_Int32() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<Int32>());
[Fact]
public void SpanBasedConstructorWithLessElements_UInt64() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<UInt64>());
[Fact]
public void SpanBasedConstructorWithLessElements_Int64() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<Int64>());
[Fact]
public void SpanBasedConstructorWithLessElements_Single() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<Single>());
[Fact]
public void SpanBasedConstructorWithLessElements_Double() => Assert.Throws<IndexOutOfRangeException>(() => TestSpanBasedConstructorWithLessElements<Double>());

private void TestSpanBasedConstructorWithLessElements<T>() where T : struct
{
T[] values = GenerateRandomValuesForVector<T>(Vector<T>.Count - 1).ToArray();
var vector = new Vector<T>(new Span<T>(values));
}

#endregion Tests for Span based constructor

#region Tests for Array based constructor

[Fact]
public void ArrayBasedConstructor_Byte() => TestArrayBasedConstructor<Byte>();
[Fact]
public void ArrayBasedConstructor_SByte() => TestArrayBasedConstructor<SByte>();
[Fact]
public void ArrayBasedConstructor_UInt16() => TestArrayBasedConstructor<UInt16>();
[Fact]
public void ArrayBasedConstructor_Int16() => TestArrayBasedConstructor<Int16>();
[Fact]
public void ArrayBasedConstructor_UInt32() => TestArrayBasedConstructor<UInt32>();
[Fact]
public void ArrayBasedConstructor_Int32() => TestArrayBasedConstructor<Int32>();
[Fact]
public void ArrayBasedConstructor_UInt64() => TestArrayBasedConstructor<UInt64>();
[Fact]
public void ArrayBasedConstructor_Int64() => TestArrayBasedConstructor<Int64>();
[Fact]
public void ArrayBasedConstructor_Single() => TestArrayBasedConstructor<Single>();
[Fact]
public void ArrayBasedConstructor_Double() => TestArrayBasedConstructor<Double>();

private void TestArrayBasedConstructor<T>() where T : struct
{
T[] values = GenerateRandomValuesForVector<T>(Vector<T>.Count).ToArray();
var vector = new Vector<T>(values);
ValidateVector(vector,
(index, val) =>
{
Assert.Equal(values[index], val);
});
}

[Fact]
public void ArrayIndexBasedConstructor_Byte() => TestArrayIndexBasedConstructor<Byte>();
[Fact]
public void ArrayIndexBasedConstructor_SByte() => TestArrayIndexBasedConstructor<SByte>();
[Fact]
public void ArrayIndexBasedConstructor_UInt16() => TestArrayIndexBasedConstructor<UInt16>();
[Fact]
public void ArrayIndexBasedConstructor_Int16() => TestArrayIndexBasedConstructor<Int16>();
[Fact]
public void ArrayIndexBasedConstructor_UInt32() => TestArrayIndexBasedConstructor<UInt32>();
[Fact]
public void ArrayIndexBasedConstructor_Int32() => TestArrayIndexBasedConstructor<Int32>();
[Fact]
public void ArrayIndexBasedConstructor_UInt64() => TestArrayIndexBasedConstructor<UInt64>();
[Fact]
public void ArrayIndexBasedConstructor_Int64() => TestArrayIndexBasedConstructor<Int64>();
[Fact]
public void ArrayIndexBasedConstructor_Single() => TestArrayIndexBasedConstructor<Single>();
[Fact]
public void ArrayIndexBasedConstructor_Double() => TestArrayIndexBasedConstructor<Double>();

private void TestArrayIndexBasedConstructor<T>() where T : struct
{
T[] values = GenerateRandomValuesForVector<T>(Vector<T>.Count * 2).ToArray();
int offset = Vector<T>.Count - 1;
var vector = new Vector<T>(values, offset);
ValidateVector(vector,
(index, val) =>
{
Assert.Equal(values[offset + index], val);
});
}

[Fact]
public void ArrayBasedConstructorWithLessElements_Byte() => TestArrayBasedConstructorWithLessElements<Byte>();
[Fact]
public void ArrayBasedConstructorWithLessElements_SByte() => TestArrayBasedConstructorWithLessElements<SByte>();
[Fact]
public void ArrayBasedConstructorWithLessElements_UInt16() => TestArrayBasedConstructorWithLessElements<UInt16>();
[Fact]
public void ArrayBasedConstructorWithLessElements_Int16() => TestArrayBasedConstructorWithLessElements<Int16>();
[Fact]
public void ArrayBasedConstructorWithLessElements_UInt32() => TestArrayBasedConstructorWithLessElements<UInt32>();
[Fact]
public void ArrayBasedConstructorWithLessElements_Int32() => TestArrayBasedConstructorWithLessElements<Int32>();
[Fact]
public void ArrayBasedConstructorWithLessElements_UInt64() => TestArrayBasedConstructorWithLessElements<UInt64>();
[Fact]
public void ArrayBasedConstructorWithLessElements_Int64() => TestArrayBasedConstructorWithLessElements<Int64>();
[Fact]
public void ArrayBasedConstructorWithLessElements_Single() => TestArrayBasedConstructorWithLessElements<Single>();
[Fact]
public void ArrayBasedConstructorWithLessElements_Double() => TestArrayBasedConstructorWithLessElements<Double>();

private void TestArrayBasedConstructorWithLessElements<T>() where T : struct
{
T[] values = GenerateRandomValuesForVector<T>(Vector<T>.Count - 1).ToArray();
Assert.Throws<IndexOutOfRangeException>(() => new Vector<T>(values));
}

[Fact]
public void ArrayIndexBasedConstructorLessElements_Byte() => TestArrayIndexBasedConstructorLessElements<Byte>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_SByte() => TestArrayIndexBasedConstructorLessElements<SByte>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_UInt16() => TestArrayIndexBasedConstructorLessElements<UInt16>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_Int16() => TestArrayIndexBasedConstructorLessElements<Int16>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_UInt32() => TestArrayIndexBasedConstructorLessElements<UInt32>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_Int32() => TestArrayIndexBasedConstructorLessElements<Int32>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_UInt64() => TestArrayIndexBasedConstructorLessElements<UInt64>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_Int64() => TestArrayIndexBasedConstructorLessElements<Int64>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_Single() => TestArrayIndexBasedConstructorLessElements<Single>();
[Fact]
public void ArrayIndexBasedConstructorLessElements_Double() => TestArrayIndexBasedConstructorLessElements<Double>();

private void TestArrayIndexBasedConstructorLessElements<T>() where T : struct
{
T[] values = GenerateRandomValuesForVector<T>(Vector<T>.Count * 2).ToArray();
Assert.Throws<IndexOutOfRangeException>(() => new Vector<T>(values, Vector<T>.Count + 1));
}

#endregion Tests for Array based constructor

#region Tests for constructors using unsupported types

[Fact]
public void ConstructorWithUnsupportedTypes_Guid() => TestConstructorWithUnsupportedTypes<Guid>();
[Fact]
public void ConstructorWithUnsupportedTypes_DateTime() => TestConstructorWithUnsupportedTypes<DateTime>();
[Fact]
public void ConstructorWithUnsupportedTypes_Char() => TestConstructorWithUnsupportedTypes<Char>();

private void TestConstructorWithUnsupportedTypes<T>() where T : struct
{
Assert.Throws<NotSupportedException>(() => new Vector<T>(new Span<T>(new T[4])));
}

#endregion Tests for constructors using unsupported types

#endregion Constructor Tests
}
}
Loading