Adding ValueTuple and ITuple to corlib in CoreRT #2399
Conversation
|
Hi @jcouv, I'm your friendly neighborhood .NET Foundation Pull Request Bot (You can call me DNFBOT). Thanks for your contribution! The agreement was validated by .NET Foundation and real humans are currently evaluating your PR. TTYL, DNFBOT; |
|
looks EditorBrowsableAttribute is not exist in corert yet. |
|
cc @jkotas |
Some related discussion on this here: https://github.com/dotnet/corefx/issues/13746#issuecomment-265792463. It looks like it's being planned to move the attribute down. |
CoreRT is going to use tests in CoreFX. (.NET Native has some old tests in TFS as well, but they are going away.) |
If you would like to add/move the EditorBrowsableAttribute to CoreLib as part of this, you are welcomed to. It needs to be added in both CoreCLR and CoreRT CoreLibs. |
|
Same comments as for the CoreCLR change - the .cs files should be exactly same between CoreCLR and CoreRT. |
|
@jkotas Regarding CoreCLR and CoreRT files, the new files I added do match now. But |
|
ok |
| @@ -198,6 +200,8 @@ | |||
| <Compile Include="System\Reflection\TypeInfo.cs" /> | |||
| <Compile Include="System\Reflection\TypeInfo.Workaround.cs" /> | |||
| <Compile Include="System\Reflection\Runtime\CustomAttributes\RuntimeImplementedCustomAttributeData.cs" /> | |||
| <Compile Include="System\TupleExtensions.cs" /> | |||
There was a problem hiding this comment.
Nit - we try to make this list sorted alphabetically.
|
@jcouv I have moved the EditorBrowsableAttribute to CoreLib. You can uncomment it. |
| @@ -8,11 +8,14 @@ namespace System.Numerics.Hashing | |||
|
|
|||
| internal static class HashHelpers | |||
| { | |||
| public static readonly int RandomSeed = new Random().Next(Int32.MinValue, Int32.MaxValue); | |||
There was a problem hiding this comment.
Thanks for the suggestion. I don't mind Int32. I'll probably leave as-is.
| /// </summary> | ||
| public T2 Item2; | ||
| /// <summary> | ||
| /// T |
There was a problem hiding this comment.
@jkotas Please advise on this question.
There was a problem hiding this comment.
Let's change it back to EqualityComparer.Default in CoreRT for now, and create issue to re-enable it once the optimization is in place.
ValueTuple types are supporting the C#7.0 tuples feature. They have been implemented as a standalone CoreFx package, but should be moved down into the various corlibs (mscorlib/desktop and mscorlib/mono are already done) to be consistent with
System.Tupleand can be used by other APIs.This PR does not contain
ITupleyet, but I will add it shortly. It is an interface inCompilerServicesnamespace used for dynamic pattern matching (in a future version of C#). It applies toSystem.TupleandSystem.ValueTupletypes. It was implemented in mscorlib/desktop, and I'm in the process of implementing it in other corlibs.The main purpose for opening this PR at this point is to raise two questions:
ValueTuplego?System.Tuple? I could not find it.FYI @tarekgh @VSadov