theraot / Theraot

Backporting .NET and more: LINQ expressions in .net 2.0 - nuget Theraot.Core available.
MIT License
162 stars 30 forks source link

Provide KeyValuePair methods. Fixes #134 #136

Closed NN--- closed 3 years ago

theraot commented 3 years ago

AppVeyor was failing on EditorBrowsableAttribute

Did we not agree that .NET 5.0 is a .NET Core? All LESSTHAN_NETCOREAPP20 targets are LESSTHAN_NET50 targets. Use TARGETS_NET.

There should be no need for "Ex" in KeyValuePairEx.

Performance is going to be bad, with Deconstruct making a copy of the struct.

AggressiveInlining?

I also found that DictonaryEntry got a Deconstruct.

Is this nullable: https://docs.microsoft.com/en-us/dotnet/api/system.collections.generic.keyvaluepair.create?view=net-5.0 I see a ? there. I think that is an error in the documentation.

See also: https://github.com/dotnet/coreclr/blob/81033d3d863004e2535e08b7b78b7dda35228253/src/mscorlib/src/System/Collections/Generic/KeyValuePair.cs

NN--- commented 3 years ago

AppVeyor was failing on EditorBrowsableAttribute

Fixed

Did we not agree that .NET 5.0 is a .NET Core? All LESSTHAN_NETCOREAPP20 targets are LESSTHAN_NET50 targets. Use TARGETS_NET. Right. My mistake :)

There should be no need for "Ex" in KeyValuePairEx.

There will be collision with the original class.

Performance is going to be bad, with Deconstruct making a copy of the struct.

AggressiveInlining?

Good question, I can add it. .NET 5.0 doesn't use this attribute.

I also found that DictonaryEntry got a Deconstruct. Sure, this can be added.

Is this nullable: docs.microsoft.com/en-us/dotnet/api/system.collections.generic.keyvaluepair.create?view=net-5.0 I see a ? there. I think that is an error in the documentation.

See also: dotnet/coreclr@81033d3/src/mscorlib/src/System/Collections/Generic/KeyValuePair.cs

It does work with nullable without any issue:

If TKey or TValue is nullable then it can hold null as expected.

using System.Collections.Generic;

#nullable enable

class P
{
    static void Main()
    {
        var kv = KeyValuePair.Create((string?) null, 1);

        (string? k, int v) = kv;
    }
}
theraot commented 3 years ago

I have added AggressiveInlining in other situations. Although, it does not have to be on the same commit.

NN--- commented 3 years ago

Added the attribute and DictionaryEntry extension.

theraot commented 3 years ago

There should be no need for "Ex" in KeyValuePairEx. There will be collision with the original class.

Will there be? KeyValuePair would not conflict with KeyValuePair<TKey, TValue> because of the generic parameters. And we don't have to add KeyValuePairEx on targets where KeyValuePair exist, only those were it does not.

NN--- commented 3 years ago

Right :)

theraot commented 3 years ago

It says there is a conflict on the tests project. It appears that Microsoft added the KeyValuePair class via nuget in some targets. I can check that later.

NN--- commented 3 years ago

Fixed. Waiting for build