[Feature request] Simplify null == null and null != null

[SpaceOgre]

Description

I'm buildning a library of Select Expressions for our product and have found that I would like to do something like this:

internal static class MapperDTO
{
    internal static Expression<Func<Subject, EducationEvent, PeriodData, MyDTO>> ToDTO =>
        (subject, educationEvent, periodData) => new MyDTO
        {
            FolkbokforingskommunName = periodData == null ? subject.Folkbokforingskommun.Name : periodData.Municipality.Name,
        };
}

This is then Invoked and Expanded with LinqKit and passed down to Entity Framework.

Expression<Func<EducationEvent, MyDTO>> mapper = 
    educationEvent => MapperForDTO.ToDTO.Invoke(educationEvent.Subject, educationEvent, null);

Obviously this does not work since null is not allowed for non-static parameters (even static?) but I have looked at the expression tree and this does create an IIF condition that looks like this:

IIF(null == null, subject.Folkbokforingskommun.Name, null.Municipality.Name)

Would it be much work to eliminate the IIF and just replace it with the correct branching?

[Thorium]

Basically this should already happen...
I expect IIF is a ConditionalExpression ?

It says that in conditional expression, visit also the test-part here

In your case, the test part, I expect to be a BinaryExpression which should be replaced to "true" due to this rule

And then when it's a true, it should be replaced via this rule

So why is it not working... Maybe there is some problem with comparing null-expression to null-expression, e.g. can they differ the type level information? Or are the Left and Right part of your BinaryExpression something else than Constants? Like "null"s here?

[SpaceOgre]

Thanks for the information and extensive reply. I will try and see if I can get more information about this tomorrow.

The IIF was probably from the ToString representation of the expression in debug mode will try and see if I can se the real ExpressionTypes instead.

[SpaceOgre]

I have looked at it now and it seems like they differ on the type level:

But even when I change to Object type it does not help:

But if I change the compare part in C# to this:

FolkbokforingskommunNameSubject = null == null ? subject.Folkbokforingskommun.Name : periodData.Municipality.Namn,

Then it is reduced so I think it have to be this line that is not working for this perticilure case:

Linq.Expression.Optimizer/src/Code/ExpressionOptimizer.fs

Line 44 in 68cf0e4

| ExpressionType.Constant, (:? ConstantExpression as ce) when (ce.Value :? IComparable) -> Some (ce.Value :?> IComparable)

I added the IComparable interface to the Class in question and now it works, I did not even have to implement the CompareTo method for it to work

public int CompareTo(object obj)
{
    throw new NotImplementedException();
}

so it seems like it is only checking if the IComparable interface exist on the type but don't use it.

It would be nice if I would not have to add that interface on my class. I'm not good at writning F# (I'm a C# dev) but would it be possible to add an extra Match just for nulls?
Maybe something like this?

ExpressionType.Constant, (:? ConstantExpression as ce) when (isNull ce.Value) -> Some (null)

[Thorium]

I think the proper special case here, is that IComparable should be needed for >=, >, '<', and <=, but the equality == and != should require IEquatable. So adding a support for IEquatable should help here.

I think it'd be a reasonable addition.

The reason I'm kind of cautious to create these are that if someone compares more complex objects than primitive types, like your PeriodData here, the likelihood of someone adding a side-effect to any cast or comparison increases, and eliminating these in LINQ will kill the side-effects away.

[SpaceOgre]

I think that sounds like a great option.

To minimize the effect of this, maybe add some kind of settings parameter, so the user can configure if the new check is desired? This would be more work of course, but it would also make sure that the new check is not used until it is op-in, and thus it would not break any existing code-bases.

In the meantime I did this:

private static Expression<Func<Subject, EducationEvent, bool, PeriodData, MyListDTO>> BaseExpression =>
    (subject, educationEvent, usePeriodData, periodData) => new MyListDTO
    {
        FolkbokforingskommunName = usePeriodData ? periodData.Municipality.Namn : subject.Folkbokforingskommun.Name,
    };

And I set the usePeriodData to true when periodData exists. By doing it like this the Optimizer will pick the correct branch in the ConditionExpression.