fsharp / fslang-suggestions

The place to make suggestions, discuss and vote on F# language and core library features
345 stars 21 forks source link

Conditional let-bound functions #846

Open Happypig375 opened 4 years ago

Happypig375 commented 4 years ago

I propose we allow conditional let-bound functions.

The existing way of approaching this problem in F# is having a helper class for the conditional method.

module M =
    type private HelperClass =
        [<System.Diagnostics.Conditional "DEBUG">]
        static member Log (x:string) = System.Diagnostics.Debug.WriteLine x

    // FS0826: The 'ConditionalAttribute' attribute may only be used on members
    let [<System.Diagnostics.Conditional "DEBUG">] private log (x:string) =
        System.Diagnostics.Debug.WriteLine x

Pros and Cons

The advantages of making this adjustment to F# are

  1. Consistency - let-bound functions in modules compile to methods. What makes them not allowed to be conditional?
  2. Conciseness - A helper class just to define a helper function?

The disadvantages of making this adjustment to F# are none that I can think of.

Extra information

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

Related suggestions: https://github.com/dotnet/fsharp/issues/8689 - In contrast, user-defined method-only attributes can be placed on let-bound properties and literals??

Affidavit (please submit!)

Please tick this by placing a cross in the box:

Please tick all that apply:

abelbraaksma commented 4 years ago

What makes them not allowed to be conditional?

I may be able to answer that one, as I've stumbled upon this myself when writing a log framework.

The issue is with let bound functions to be composable and curryable. The CLI spec requires that a conditional method have void return (otherwise, how can you safely remove every appearance of the method, and each of its arguments?). The moment you curry or compose, you create a new function that returns a function, which is, indeed, not void anymore. If F# were to implement this, it would have to track all usages, including parameters passing and exposures.

Hence it's not possible to implement this, unless we would severely limit the allowed usages of conditional let bound functions. That is: disallow construction, composition, passing, currying. Or only allow a single argument.

But since that's basically how the current feature works (methods have the above restrictions, that is, they have only one argument), I think that's why they haven't gone through the trouble of trying to implement this (see F# spec, it's explicitly mentioned there).

I think there's a language suggesting from me out there from a few years back, where @dsyme or @cartermp gave me some version of above explanation.

I'm not against the proposal itself, but I fear it's going to be very complex to implement (provided we get the rules right), and then it's a question as to whether it's worth it.

Happypig375 commented 4 years ago

@abelbraaksma Wdym


Microsoft (R) F# Interactive version 10.7.0.0 for F# 4.7
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> type C = [<System.Diagnostics.Conditional "DEBUG">] static member X (x:int) (y:int) = printfn "Hi";;
type C =
  class
    static member X : x:int -> y:int -> unit
  end

> C.X 1 2;;
val it : unit = ()

> C.X 1;; // Partial application works just fine
val it : (int -> unit) = <fun:it@4>

> (C.X 1) 2;;
val it : unit = ()

> type C = [<System.Diagnostics.Conditional "DEBUG">] static member X (x:int) (y:int) = printfn "Hi"; 2;;
type C =
  class
    static member X : x:int -> y:int -> int
  end

> C.X 1 2;; // Only conditional methods that do not return unit errors

  C.X 1 2;;
  ^^^^^

stdin(6,1): error FS0001: This expression was expected to have type
    'unit'    
but here has type
    'int'    

> 
abelbraaksma commented 4 years ago

// Partial application works just fine

You created a new function that returns a function, not a new conditional member. The new function cannot have the conditional attribute, because it doesn't return void. Your code shows precisely what I meant.

It also shows how tricky it can be to know whether any given function will be erased. There are cases where the compiler correctly notices a curried function (from a member) to have the conditional attribute, and will get erased. Though I'm unsure that's due to the conditional attribute, or because the compiler already knows to erase functions that do nothing (try piping into fun _ -> (), and similar, and check the optimized IL). In this case, it would have to know at compile time that the member has no side effects.

I've experimented a lot with this in the past (and it's possible the compiler has gotten smarter since), so far, the only safe bet seems to be to stick with members decorated with the conditional attribute.

Happypig375 commented 4 years ago

You created a new function that returns a function, not a new conditional member. The new function cannot have the conditional attribute, because it doesn't return void. Your code shows precisely what I meant.

Well, no.

> type C = [<System.Diagnostics.Conditional "DEBUG">] static member X (x:int) (y:int) = printfn "Hi";;
type C =
  class
    static member X : x:int -> y:int -> unit
  end

> type B = [<System.Diagnostics.Conditional "DEBUG">] static member X (y:int) = C.X 1;;
type B =
  class
    static member X : y:int -> (int -> unit)
  end

> B.X 4;;

  B.X 4;;
  ^^^

stdin(20,1): error FS0001: This expression was expected to have type
    'unit'    
but here has type
    'int -> unit'    

It's the usage that is prohibited. Like my C.X 1 2;; // Only conditional methods that do not return unit errors above.

Happypig375 commented 4 years ago

The compiler can just as easily generate a lambda wrapping the conditional function at the call site if not all parameters are provided.

abelbraaksma commented 4 years ago

Yes, it can, but that violates the whole idea of conditional members: they are designed to be completely erased, including any arguments, including any variables or calls that create those arguments and so on. It all disappears.

That's a CLI spec rule, and while I agree that your idea in your last comment is technically feasible, it would violate this guarantee, which would violate the whole principle: a conditional member gets erased, completely (on the call site, of course). And as soon as there's a return value, that's not possible.

But let's assume for a moment that the compiler can track all lambdas and erase them. Then you still have to deal with the kind of functions that always return a value, like sprintf style functions with kprintf (I tried to make it work with members and conditionals, not possible, unless you tuple all arguments), or those that return stuff like Unchecked.defaultOf<'T>.

Anyway, I'd love to find a solution for all this, esp for the sprintf style stuff, because the reason that you can't write a conditional trace or log library that uses that magic syntax is precisely because your proposed feature is not there (and I guess it's not there because it's very hard to define in pure functions, at least that's what I think based on earlier discussions some years ago).

Happypig375 commented 4 years ago

they are designed to be completely erased, including any arguments, including any variables or calls that create those arguments and so on. It all disappears.

Huh. Trying it in the F# Interactive:

Microsoft (R) F# Interactive version 10.7.0.0 for F# 4.7
Copyright (c) Microsoft Corporation. All Rights Reserved.

For help type #help;;

> type C = [<System.Diagnostics.Conditional "DEBUG">] static member F (x:int) = ();;
type C =
  class
    static member F : x:int -> unit
  end

> C.F (printfn "Argument Evaluated"; 3);;
val it : unit = ()

> type C = [<System.Diagnostics.Conditional "DEBUG">] static member F (x:int) (y:int) = ();;
type C =
  class
    static member F : x:int -> y:int -> unit
  end

> C.F (printfn "Argument Evaluated"; 3) 2;;                                                 
Argument Evaluated
val it : unit = ()

This is only partially true.

Happypig375 commented 4 years ago

If this rule is to be enforced, we could require passing all parameters at the call site, disabling partial application.

abelbraaksma commented 4 years ago

This is only partially true

The statement from the spec is completely true, but you created a partial function, that in itself wasn't decorated. And that's precisely the point here.

Happypig375 commented 4 years ago

The statement from the spec is completely true, but you created a partial function, that in itself wasn't decorated. And that's precisely the point here.

That function is decorated. What are you talking about?

type C = [<System.Diagnostics.Conditional "DEBUG">] static member F (x:int) (y:int) = ();;
let f x = C.F (printfn "Argument Evaluated"; 3) 2; 4
using Microsoft.FSharp.Core;
using System;
using System.Diagnostics;
using System.IO;
using System.Reflection;

[assembly: FSharpInterfaceDataVersion(2, 0, 0)]
[assembly: AssemblyVersion("0.0.0.0")]
[CompilationMapping(SourceConstructFlags.Module)]
public static class _
{
    [Serializable]
    [CompilationMapping(SourceConstructFlags.ObjectType)]
    public class C
    {
        [Conditional("DEBUG")] // <------ Declaration
        [CompilationArgumentCounts(new int[] {
            1,
            1
        })]
        public static void F(int x, int y)
        {
        }
    }

    public static int f<a>(a x)
    {
        PrintfFormat<Unit, TextWriter, Unit, Unit> format = new PrintfFormat<Unit, TextWriter, Unit, Unit, Unit>("Argument Evaluated");
        PrintfModule.PrintFormatLineToTextWriter(Console.Out, format);
        int num = 3; // <---- for some reason these are still present
        return 4;
    }
}
namespace <StartupCode$_>
{
    internal static class $_
    {
    }
}
abelbraaksma commented 4 years ago

That function is decorated. What are you talking about?

Apologies, I didn't mean to upset you in any way. It looks like I failed to be clear in my previous posts, or maybe we are talking about different things. Something like that ;).

Related, I found this SO question from myself, and more recently a discussion here on github for a dprintf function: https://github.com/dotnet/fsharp/issues/8522. It adds some background, but isn't conclusive.

Here's what the spec says (section 14.4.2):

If a member definition has the System.Diagnostics.Conditional attribute, then any application of the member is adjusted as follows:

  • The Conditional("symbol") attribute may apply to methods only.
  • Methods that have the Conditional attribute must have return type unit. The return type may be checked either on use of the method or definition of the method.
  • If symbol is not in the current set of conditional compilation symbols, the compiler eliminates application expressions that resolve to calls to members that have the Conditional attribute and ensures that arguments are not evaluated. Elimination of such expressions proceeds first with static members and then with instance members, as follows:
    • Static members: Type.M(args) ➔ ()
    • Instance members: expr.M(args) ➔ ()

And here's my interpretation of that, based on previous experience, and after looking at how I structured my code in my logging framework:

To make this behavior more tangible, try the following:

type E = 
    [<Conditional "DEBUG">] 
    static member F (x: int) = fun _ -> ();;

// gives: static member F : x:int -> ('a -> unit)

Calling this will have the compiler complain:

> E.F 42 5;;

  E.F 42 5;;
  ^^^^^^

stdin(36,1): error FS0001: This expression was expected to have type
    'unit'    
but here has type
    ''a -> unit'  

Remove the ConditionalAttribute and call again, it'll "just work" as expected.

What happened here?

  1. The compiler accepts the member (though I believe an error here would've been better)
  2. Upon calling, it first evaluates E.F 42, which is decorated with Conditional.
  3. This doesn't return unit, it returns a function, so it throws a (rather cryptic, I must say) error

There's no way of calling such method. But in the case of F (x: int) (y: int) = (), the function is not explicitly created, but created by the compiler. The signature is F: int -> int -> unit, which is the same as F: (int -> int) -> int -> unit. I would very much like an error here too, as it is very confusing that the intermediate function is not decorated. If it was, like in my (explicit) example above, it would have given a similar error for clarity's sake.


Back to the issue at hand. The spec says, explicitly, "ensures that arguments are not evaluated". It doesn't, however, explicitly say what it means with "arguments". So let's try something:

type C = [<Conditional "DEBUG">] static member F x y = printfn "inside %A, %A" x y
type E = [<Conditional "DEBUG">] static member F (x, y) = printfn "inside %A, %A" x y

[<Test>]
let ``Test conditional symbol 1``() =
    (id E.F)(printfn "one", printfn "two")  // prints one two

[<Test>]
let ``Test conditional symbol 2a``() =
    let x = (printfn "one", printfn "two")  // prints one two, correct
    E.F x

[<Test>]
let ``Test conditional symbol 2b``() =
    let x = (printfn "one", printfn "two")  // prints one two, correct
    E.F (fst x, snd x)

[<Test>]
let ``Test conditional symbol 3a``() =
    E.F(printfn "one", printfn "two")  // prints nothing, correct

[<Test>]
let ``Test conditional symbol 3b``() =
    let p() = (printfn "one", printfn "two")  // BUG: should not print anything, but prints one two
    E.F(p())

[<Test>]
let ``Test conditional symbol 3c``() =
    let p() = (printfn "one", printfn "two")  // prints nothing, correct
    E.F(fst(p()), snd (p()))

[<Test>]
let ``Test conditional symbol 4``() =
    (printfn "one", printfn "two") |> E.F  // prints one two, correct

[<Test>]
let ``Test conditional symbol 5``() =
    C.F (printfn "one") (printfn "two")  // prints one two, correct

[<Test>]
let ``Test conditional symbol 6``() =
    let x = (printfn "one", printfn "two")  // prints one two, correct
    C.F (fst x) (snd x)

None of the tests above will print "inside...", and only those calls where the tuple form is used gets the whole argument-evaluation eliminated (except for "3b", which I believe is a bug).

dsyme commented 4 years ago

I agree this should be addressed. There are a number of these things that work only for members and not for let or vice-versa.