dotnet / fsharp

The F# compiler, F# core library, F# language service, and F# tooling integration for Visual Studio
https://dotnet.microsoft.com/languages/fsharp
MIT License
3.87k stars 783 forks source link

"inline" keyword not accepted next to "fun" keyword #467

Closed exercitusvir closed 9 years ago

exercitusvir commented 9 years ago

Repost of issue Functions defined as a value can't be inlined where Don Syme suggested to post it here.

The following two definitions are semantically identical and compiled to exactly the same code:

let add (x : int) (y : int) : int = 
   x + y

let add : int -> int -> int =
    fun x y -> x + y

Yet the inline keyword is not allowed next to fun while it is allowed next to let.

That is:

let inline add (x : int)  (y : int) : int = 
   x + y

compiles, but neither

let add : int -> int -> int =
    inline fun x y -> x + y

nor

let add : int -> int -> int =
    fun inline x y -> x + y

compiles.

I consider this a bug because

let add : int -> int -> int =
    fun x y -> x + y

is valid syntax for defining a function, but making it inline is not supported. It should not make a difference which style I choose. Especially in terms of maintainability - right now - I would have to rewrite the function to add inline which is a considerable and unnecessary inconvenience.

latkin commented 9 years ago

Interesting idea. This is definitely in "feature request" territory IMO. Note that this issue and the linked uservoice don't agree on the proposed syntax.

The syntax suggestion proposed here makes more sense to me, though, since the lambda function can potentially close over non-inline data. It might be part of a function itself, too, or represent a non-lambda.

// confusing - is 'cache' inlined somehow, with multiple copies?
let inline expensiveFunc =
    let cache = Dictionary<_,_>()
    fun x -> ... memoized ...

// inline here should apply to...?
let inline freshExpensiveFunc () =
    let cache = Dictionary<_,_>()
    fun x -> ... memoized ...

// what does this mean?
let inline printList =
    List.iter (printfn "%A")

// clear what is happening
let expensiveFunc =
    let cache = Dictionary<_,_>()
    inline fun x -> ... memoized ...

One downside of attaching inline next to fun is that it could be applied in useless places (e.g. [1..3] |> List.map (inline fun x -> x + 1)). Maybe the compiler could restrict this to L-values only.

allykzam commented 9 years ago

The "bug" mindset I originally had was based on the notion that in functional programming, a function should always be a value anyway, so defining it as x = λ y → ... should be no different than x y = .... For the sake of prioritization, I completely agree with calling it a new feature.

While the originally proposed syntax makes more sense to me, I also like the new syntax because it means I could do this:

let processThings =
    let f = inline fun x -> ...
    let a = f 7
    let b = f 8
...

Could be helpful for a short lambda that will get used a handful of times. Perhaps an opportunity for lots of premature optimization, but I can see it being more useful to me with that form.

Thanks to the recent CLA change, I'd be happy to take a look sometime before vNext if nobody else wants to work on it.

exercitusvir commented 9 years ago

I also like inline fun syntax the most.

One downside of attaching inline next to fun is that it could be applied in useless places (e.g. [1..3] > |> List.map (inline fun x -> x + 1) ). Maybe the compiler could restrict this to L-values only.

It might not actually be useless. If List.map was inline, then fun x -> x + 1 would also be inlined because inline works recursively. I think I also read somewhere that the compiler does not always inline a function that is marked as inline, so it's basically just a hint that it would be a good idea to inline it and the compiler respects it most of the time.

The "bug" mindset I originally had was based on the notion that in functional programming, a function > should always be a value anyway, so defining it as x = λ y → ... should be no different than x y = ... . For the sake of prioritization, I completely agree with calling it a new feature

I still regard this as a bug in terms of prioritization. It's makes the F# syntax inconsistent. That several people tried to put inline on a lambda is already an indication that this should be possible.

I've also never been a fan of putting parenthesis around parameters to specify the types. It makes the syntax irregular and ugly and parenthesis are already way too overloaded. I even like the alternative F# syntax for function definitions, that is:

let add : int -> int -> int =
fun x y -> x + y //this should not issue a warning with regards to indendation though

more than the equivalent Haskell definition with type annotations:

add :: int -> int -> int 
add x y = x * y

because name of the function is not repeated and let makes it explicit that this is a definition. It somehow looks cleaner to me and way readable than:

let add (x : int)  (y : int) : int = 
   x + y

I also find the alternative syntax more consistent in general. let is always only followed by the identifier of function like ordinary value bindings (unless marked as inline) and there is no difference in definition between named functions and anonymous functions. It's also more consistent with regards to the specification of abstract members and statically resolved member constraints. And I like that there is a clear visual separation between function definition and function application, which looks identical if you leave off let. I find so many advantages that I would even propose this as preferred syntax.

Thanks to the recent CLA change, I'd be happy to take a look sometime before vNext if nobody else > wants to work on it.

That would be so great!

If you do implement this, then it would be really awesome if it would also be possible to automatically convert existing function definitions to this syntax and even generate the type annotations from the inferred types for functions that don't have type annotations by right-clicking on a function. One might even want to automatically align the parameter type annotations and parameter names, but I would be already very happy if inline was simply allowed and supported.

allykzam commented 9 years ago

If you do implement this, then it would be really awesome if it would also be possible to automatically convert existing function definitions to this syntax

That sounds very much like a refactoring tool, which would be better put in fsprojects/VisualFSharpPowerTools, but for a (currently) non-standard syntax, I doubt a PR for that would be accepted. Not that it wouldn't be useful for the few of us using this syntax.

//this should not issue a warning with regards to indendation though

I disagree; anywhere that you currently let x = fun -> ... or let x = function ... you can't add a line-break without also indenting; it's not as nice as the haskell syntax I was trying to duplicate, but it's more consistent with the rest of F#. If this used the val add : int -> int -> int syntax that fsi files already use (someone proposed it in my original uservoice post), I think it would make more sense, but adding support for that syntax seems like it'd be a much bigger change thank just fixing how the compiler views function values. :)

@latkin: Do you (or anyone else) have any suggestions where I should start looking at this? Or should I just try debugging the compiler and giving it code to chew on, following its execution path?

exercitusvir commented 9 years ago

That sounds very much like a refactoring tool, which would be better put in fsprojects/VisualFSharpPowerTools, but for a (currently) non-standard syntax, I doubt a PR for that would be accepted

You're right. I'd already be ecstatic if the syntax was allowed.

I disagree; anywhere that you currently let x = fun -> ... or let x = function ... you can't add a line-break without also indenting;

That's a good point. That would make it less consistent.

If this used the val add : int -> int -> int syntax that fsi files already use

I agree. This would be even nicer.

drvink commented 9 years ago

Just throwing in my two cents: I'd been meaning to file an issue that you couldn't do "let inline fn = function | ...cases..." for a while, and this issue covers that, so I agree it would be a nice thing to have.

latkin commented 9 years ago

Looks like this is being tracked at http://fslang.uservoice.com/forums/245727-f-language/suggestions/6237585-allow-inline-keyword-in-the-case-let-f-fun-a

exercitusvir commented 8 years ago

Could you please re-open this issue? Otherwise, it is no longer tracked anywhere. The request on user voice has been declined with the reason that it is being tracked here, but Don Syme would still accept it if there was an implementation without issues.

exercitusvir commented 8 years ago

@amazingant Are you still open to working on this (just the language support)?

allykzam commented 8 years ago

@exercitusvir Between the very delayed and useless response we got here, and the other responses I've gotten on other issues, I'm not keen on the idea of trying to work on a Microsoft-controlled codebase like this one. If you can get some useful feedback on how to proceed, I'd certainly reconsider, but the compiler is a confusing enough mess that I feel my time is better spent elsewhere. 😬

21c-HK commented 8 years ago

@amazingant I agree that it is sometimes frustrating, but I think they are currently just a bit overwhelmed by the number of feature requests and other issues since going open source. The F# team is still pretty small compared to the C#-team. I understand that they need to make priorities due to limited resources and that they may have other priorities than others, and most of the time the F# team makes very good decisions. I still have the feeling that Microsoft in Redmond is not really treating F# as a first-class language like C# or VB (e.g. NET Native initially not supporting F#, no XAML editor for F#, missing code samples in MSDN, F# not marketed as much as C# or VB). I was also browsing "prim-types.fs" in FSharp.Core yesterday to understand how something works and beyond being more than 6000(!) lines of code in a a single file there are essential comments missing that should explain certain design decision (which I found out about in discussions on github or codeplex).

But I still believe that F# technical superiority makes it still worth it to invest in the technology and community. The F#-team also tends to be more open to implement features than C# or Java. I also have the feeling that F# being open source has speed up development on it, which seemed to have slowed down until the F# 4.0 release.

brodyberg commented 8 years ago

@amazingant @exercitusvir @21c-HK Yes, to address this issue the community is being call upon to supply a PR. The uservoice comment specifically says it would be welcome. This repo accepts world-class PRs all the time. Follow the PR guidelines and submit one. Doing such a thing is no small piece of work, but absolutely would be evaluated on the merits.

allykzam commented 8 years ago

@21c-HK I don't doubt that the F# team is small, and I don't doubt their ability to make good decisions, but I personally have better things to do with my time than try to figure out which 6K LoC file to start with. Especially now that I have Pokemon Go. :smile: :wink:

I love F# and I love using it at work (where I actually do use it with XAML, there are just some...minor hiccups?). But when I'm unfamiliar with the codebase, I'm more likely to make PRs to a project like Paket or FAKE, where asking for help gets a useful response. Here I get silence and then someone closes my issue, or @brodyberg tells me to submit a "world-class PR" when I still have no clue where to start.

C'est la vie? ¯\_(ツ)_/¯

21c-HK commented 8 years ago

@amazingant I agree that refactoring the F# compiler to improve maintainability and readability would make it much more likely that outsiders would even consider working on the compiler. I haven't looked at the compiler code, but I imagine it being very complex (e.g. type inference) even if it was optimized for maintainability. A first step might be to refactor the code into smaller files (having seen FSharp.Core only). This would be a long-term effort that probably nobody really wants to work on (since it requires change to complex code that everyone relies upon and the benefits are not immediately visible like "look, I implemented the long-requested feature X").

I did a quick search on the issues on this repository for "compiler", "clean-up" and "refactor" each, but could not find an issue for that (e.g. "make F# compiler more maintainable"). Maybe you could open an issue like that and give a few pointers on what to improve exactly since you have look at the compiler.

@latkin I would also like to see this feature implemented in F# (whoever decides to implemented it). Is there a chance that you could re-open the issue so that it has more visibility?

dsyme commented 8 years ago

@amazingant @exercitusvir @21c-HK I'm ok with this this adjustment being made to the language (though I don't particularly favour its use to write F# code in a non-idiomatic style trying to emulate styles of other languages). So I've changed the status of the issue on uservoice to "approved" - though I don't personally plan to work on the adjustment.

I agree with @brodyberg above that the way to address this is for someone to propose a PR. I suggest starting by finding the place where the current error is raised, e.g. by searching FSComp.txt or other files for the error message text.