Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve interop to Nullable optional parameters #774

Closed
5 tasks done
dsyme opened this issue Jul 25, 2019 · 9 comments
Closed
5 tasks done

Improve interop to Nullable optional parameters #774

dsyme opened this issue Jul 25, 2019 · 9 comments

Comments

@dsyme
Copy link
Collaborator

dsyme commented Jul 25, 2019

I propose we improve interop to C#-defined Nullable-typed optional parameters

Consider a C# definition

        public int SomeMethod(int? channels = 0, int? ratio = 1)

The compiled form of this uses Nullable typed values for the parameters with default value nullref.

We do not recognise this in F# and when calling and providing a value we require

    C.SomeMethod(channels = Nullable 3)
    C.SomeMethod(channels = Nullable 3, ratio = Nullable 3)
    C.SomeMethod(ratio = Nullable 3)

Alternatives that achieve the same effect are

  1. to allow auto-conversion from T to Nullable<T> at method calls. Indeed this is probably the best solution.

  2. some even more aggressive auto-conversion rule for T to Nullable<T>

Pros and Cons

The advantages of making this adjustment to F# are interop with modern C# becomes smoother.

The disadvantages of making this adjustment to F# are

  • another implicit conversion rule is added.
  • we have to be careful this is not a breaking change

Extra information

Estimated cost (XS, S, M, L, XL, XXL): S

Related suggestions: (put links to related suggestions here)

Affidavit (please submit!)

Please tick this by placing a cross in the box:

  • This is not a question (e.g. like one you might ask on stackoverflow) and I have searched stackoverflow for discussions of this issue
  • I have searched both open and closed suggestions on this site and believe this is not a duplicate
  • This is not something which has obviously "already been decided" in previous versions of F#. If you're questioning a fundamental design decision that has obviously already been taken (e.g. "Make F# untyped") then please don't submit it.

Please tick all that apply:

  • This is not a breaking change to the F# language design
  • I or my company would be willing to help implement and/or test this
@dsyme
Copy link
Collaborator Author

dsyme commented Jul 25, 2019

There is also this case when interoperating with C#-defined optionals, where the use of an F# Some value doesn't work as expected:

image

Here the compiled form is:

.method public hidebysig static class Tensorflow.Tensor 
        'add'(class Tensorflow.Tensor x,
              class Tensorflow.Tensor y,
              [opt] string name) cil managed
{
  .param [3] = "Add"

and the C# form is

        public static Tensor add (Tensor x, Tensor y, string name = "Add")
        {
            var dict = new Dictionary<string, object>();
            dict["x"] = x;
            dict["y"] = y;
            var op = _op_def_lib._apply_op_helper("Add", name: name, keywords: dict);
            return op.output;
        }

@dsyme
Copy link
Collaborator Author

dsyme commented Jul 30, 2019

@cartermp We have a compatibility problem with this

Given this:

        public int SomeMethod(int? channels = 0, int? ratio = 1)

We currently require Nullable:

    C.SomeMethod(ratio = Nullable 3)

However the natural and "morally correct" thing is to require non-nullable, e.g.

    C.SomeMethod(ratio = 3)

The problem is that, if the method is not overloaded, we use a "strong" assumption of Nullable, e.g.

let f x = C.SomeMethod(ratio = x)

will infer Nullable<int> for the type of x. We really want this to be int. The underlying technical question is this:

what known type do we use for the checking of expr in C.M(arg=expr) when M is not overloaded and arg is a C# optional argument with type Nullable<ty>? Do we check expr with known type Nullable<ty> or ty?

Here's an example of a line that fails to compile with a type-adjusted "strong" rule. This also affects unnamed arguments, e.g. this line fails to compile

I'm not really sure what to do about this. The current strong assumption of Nullable is not what we want long term. I don't mind a type-directed rule of "if the argument expression has type Nullable (after checking it) than accept it", but such a rule would not kick in for cases like the one I mention above.

What do you think our position should be on this? The choices would be:

  1. Make a breaking change (under the /langversion switch) and use a known type of ty instead of Nullable<ty> for the argument expression. This would require users to adjust their code to remove Nullable. We would put in some adhoc rules such as "if the argument is syntactically Nullable someExpr then allow it" but this couldn't capture all cases

  2. Make a less serious breaking change and remove the propagation of a "strong" known type into the argument altogether (if the argument is a C#-style nullable optional argument). This would allow the expression to have either type Nullable<ty> or ty and we would work things out after the fact using a type-directed rule. This is almost certainly the most balanced option, as it's hard to image situations where the known type will be relevant to checking the argument expression.

    This would mean that

     let f x = C.SomeMethod(ratio = x)
    

    would continue to typecheck as today (x has type Nullable<int>) until you add a type annotation to x

  3. Don't make a change here at all.

My gut feeling is that approach (2) would be adequate.

@cartermp
Copy link
Member

cartermp commented Jul 30, 2019

My main concern here, though I think this is worth doing, is that usage of this kind of code is much more widespread than I thought (due to interfacing with DBs). It'd be good to get opinions from folks like @Thorium and others on this.

@Thorium
Copy link

Thorium commented Jul 30, 2019

Initially I have more questions than opinions:

So you mean implicit conversion from T to Nullable<T>, but not from Nullable<T> to T (like in&out in C#, the covariance/contravariance rules)?

So far the standard F#-type for maybe-monad has been Option<T>, and I don't like the way the maybes are multiplying (Option, ValueOption, Nullable, what next). So could they interop somehow better with each other, rather than implicit conversions to the base types?

In C#, null breaks SOLID principles by playing two roles:

  1. the special case pattern (GoF/Fowler),
  2. the un-initialized variable (on purpose or by accident)

Which one is this solution focusing to?

Will this cause backward compatibility issues?

Edit:
I would vote for option 3 unless this change makes F# a better programming language by its own standards.

@dsyme
Copy link
Collaborator Author

dsyme commented Jul 31, 2019

So you mean implicit conversion from T to Nullable, but not from Nullable to T (like in&out in C#, the covariance/contravariance rules)?

No, this is not about a conversion but about proper treatment of nullable-typed optionals coming from C# code.

@Thorium
Copy link

Thorium commented Jul 31, 2019

Ok, I know mostly about SqlProvider, which is pure F#, not using C# Nullables, maybe other Sql tools are affected more.

@xp44mm
Copy link

xp44mm commented Sep 18, 2019

Nullable let me confusion:

let x = Nullable(3)
let ty = x.GetType()

Assert.NotEqual(typeof<Nullable<int>>,ty)
Assert.Equal(typeof<int>,ty)

example 2:

Assert.Throws<System.NullReferenceException>(fun() -> 
    let ty = Nullable().GetType()
    ())
|> ignore

those is auto drop the Nullable, Contrary to intuition. may i get the right type Nullable<int> from object x?

@cartermp
Copy link
Member

cartermp commented Nov 8, 2020

Closing out as completed for F# 5.

@cartermp cartermp closed this as completed Nov 8, 2020
@abelbraaksma
Copy link
Member

those is auto drop the Nullable, Contrary to intuition. may i get the right type Nullable from object x?

@xp44mm, I know it’s been two years, but for posterity and others interested: this is just how GetType() works in .NET. Use IsGenericType and iterate over the type parameters to get the full definition. Here’s one way to get the ‘friendly name’ you seem to be after: https://stackoverflow.com/questions/4185521/c-sharp-get-generic-type-name

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants