API Proposal: ECPoint.IsEmpty

[vcsjones]

Proposal:

namespace System.Security.Cryptography {
    public struct ECPoint {
        // Existing
        public byte[]? X;
        public byte[]? Y;

        // New:
        [MemberNotNullWhen(false, nameof(X), nameof(Y))]
        public readonly bool IsEmpty => X == null && Y == null;
    }
}

Since the intention of PR #33874 is to allow ECParameters to have an empty Q, this would be a nice addition to allow !Q.IsEmpty instead of writing Q.X != null && Q.Y != null all over the place.

[Dotnet-GitSync-Bot]

I couldn't figure out the best area label to add to this issue. Please help me learn by adding exactly one area label.

[vcsjones]

The MemberNotNullWhen might need to be broken up in to

[MemberNotNullWhen(false, nameof(X))]
[MemberNotNullWhen(false, nameof(Y))]

For CLS compliance.

[ghost]

@bartonjs

[bartonjs]

I feel like a readonly should be in that property somewhere (might require using a block body), otherwise, looks review-worthy to me.

[vcsjones]

@bartonjs I never remember that C# feature exists. Amended. (It seems to work fine with an expression-bodied property).

[vcsjones]

@bartonjs Actually, I'm not sure this proposal as-is is nullable friendly.

Consider:

[MemberNotNullWhen(false, nameof(X), nameof(Y))]
public readonly bool IsEmpty => X == null && Y == null;

This is incorrect because false will be returned if X is null, but Y is not. We could change && to ||, which then will leave the nullable annotation correct, but IsEmpty return an unexpected result.

Perhaps a better way to do this is to make the property IsNotEmpty (yucky name up for debate).

[MemberNotNullWhen(true, nameof(X), nameof(Y))]
public readonly bool IsNotEmpty => X != null && Y != null;

[bartonjs]

Seems like maybe that's HasValue, then? But I'm not sure that HasValue... has value. (Or, really, that it's self-descriptive-enough.

Something like Validate() is OK, except that given that the whole point is that Q is now effectively optional that it's weird that Validate() returning false is sometimes valid.

So perhaps it's just best off as it is.

[vcsjones]

Perhaps the name HasPoint?

[MemberNotNullWhen(true, nameof(X), nameof(Y))]
public readonly bool HasPoint => X != null && Y != null;

"Point" conveys that both X and Y are there (since you cannot have a point with either just X or Y).

[bartonjs]

if (point.HasPoint)? It still feels a little weird to me, but at least it's somewhat describing the case.

@stephentoub Do you have thoughts here?

[vcsjones]

This was a small nice-to-have, but I don't think this is worth pursuing given that it's not entirely straight-forward as initially thought.