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

Discussion: Local Functions #2930

Closed
19 of 30 tasks
gafter opened this issue May 20, 2015 · 76 comments
Closed
19 of 30 tasks

Discussion: Local Functions #2930

gafter opened this issue May 20, 2015 · 76 comments

Comments

@gafter
Copy link
Member

gafter commented May 20, 2015

In #259 it is proposed to add local functions and types. The Roslyn compiler team met today to look at local functions (not local types) in some detail, in preparation for some planned prototyping work by @khyperia. The idea is to be able to define functions in block scope. The C# language design team will consider local functions tomorrow. If this experiment is successful we would look at VB as well. Here are my notes from the compiler team meeting (organized mainly by topic, not chronologically)

Local Functions

We discussed a number of use cases that motivated our investigation:

  • It is very common to write a helper method that is only used from one place, but it makes the code less clear to the reader because the association between the helper function and the thing it is helping is not explicit. Local functions makes the association part of the syntax.
  • An iterator method that validates its arguments (write a non-iterator function to validate the arguments, and then return the result of invoking a local iterator function)
  • A Task-returning method that has no async machinery in the common case where it doesn't need to "go async"
  • You are writing a method that returns an array, but want the syntactic convenience of the iterator method syntax (yield return).

Most of the design questions have seemingly obvious answers.

  • Syntax Trees: New production that is a new kind of block statement
  • Semantics: See below
  • Return type inference: probably allow if function does not reference itself. Need a syntax.

Feature Interactions

  • Caller info attributes on parameters
  • Local functions may be generic
  • Callers may use optional/default/named arguments
  • May capture variables (like a lambda)
  • Definite assignment just like lambda
  • May be self-recursive (if not inferred return type)
  • Inferred return type (if does not reference itself)
  • Conversions, extension methods, operators (not allowed)
  • May use {} or => syntax for function body
  • Is a method group that may be subject to delegate conversion
  • Non-local control transfer (disallow, as we do for lambdas)
  • Iterator functions supported
  • Async functions supported

Scoping Rules

  • Exactly the same as for local variables (therefore no overloading)
  • They may be declared only directly within a block

Parameters:

  • optional/default parameter values
  • ref, out
  • params
  • __arglist
  • extension ("this") not allowed
  • Caller info attributes OK

Modifiers:

  • public
  • unsafe
  • Attributes (perhaps not allowed on the function)
  • async
  • partial
  • static
  • virtual
  • extern (maybe)

Implementation and API

  • In the bound trees, add a new property on BoundBlock for function symbols declared
  • They are MethodSymbols, with symbol.MethodKind==MethodKind.LocalFunction.
  • Probably existing lambda lowering pass can be adapted to lower local functions too

Possible Future Optimization:

  • Capture by copy when possible
  • Capture by ref parameter
  • Inline into call site

/cc @khyperia @MadsTorgersen @jaredpar @agocke @AlekseyTs @VSadov @mattwar @KevinRansom @ljw1004 @AnthonyDGreen

@gafter
Copy link
Member Author

gafter commented May 20, 2015

You are writing a method that returns an array, but want the syntactic convenience of the iterator method syntax (yield return)

That would look something like this

    Foo[] GetFoos(Whatever a)
    {
        IEnumerable<Foo> result() // iterator local function
        {
            yield return a.First;
            yield return a.Last;
        }

        return result().ToArray();
    }

@tmat
Copy link
Member

tmat commented May 20, 2015

Should we do iterator lambdas in C# first?

@agocke
Copy link
Member

agocke commented May 20, 2015

@tmat No, iterator lambdas don't provide much value -- you end up with a delegate that returns an IEnumerable, which is almost always worse than a local iterator function.

@agat50
Copy link

agat50 commented May 20, 2015

As i mentioned in https://visualstudio.uservoice.com/forums/121579-visual-studio/suggestions/6504697-block-code-as-value-another-syntax-for-semicolon would be nice to see

Foo[] GetFoos(Whatever a)
{
    return {
        yield return a.First;
        yield return a.Last;
    }.ToArray();
}

Imho, local functions are using one time mostly, so no needs for name\parameters\return type. If we need multiple invocation, lambda converted to Func\Action is enough already.

@paulomorgado
Copy link

void M()
{
    var f(int n)
    {
        return (n == 1) ? 1 : n * f(n - 1);
    }

    var x = f(5);
}

Can't the return type of f be inferred on this case?

@paulomorgado
Copy link

Will expression bodied funtions be allowed?

void M()
{
    var f(int n) => (n == 1) ? 1 : n * f(n - 1);

    var x = f(5);
}

@svick
Copy link
Contributor

svick commented May 20, 2015

@agat50 I think named local functions are useful, the lambda syntax can be awkward (and doesn't work well with recursion).

Though your suggestion is not without merit either. I've wanted something similar for async blocks executing concurrently (but not necessarily using multiple threads). E.g.:

var task1 = async
{
    Console.WriteLine(await FooAsync());
};
var task2 = async
{
    Console.WriteLine(await BarAsync());
};

await Task.WhenAll(task1, task2);

Though I guess you could achieve a very similar effect with a library method.

@ljw1004
Copy link
Contributor

ljw1004 commented May 20, 2015

Lambdas work great in VB already for all the use-cases in this thread.

Here's Neal's initial example, which in VB involves less repetition than Neal's C# version since you don't need to write the return type of the lambda. (@agocke you said it's worse than the local function but I don't see any way that it's worse).

Function GetFoos(a As Whatever) As Foo()
    Dim result = Iterator Function()
                     Yield a.First
                     Yield a.Last
                 End Function
    Return result().ToArray
End Function

For @agat50 who liked iterator blocks, here's how you'd do them with iterator lambdas, again involving no more syntactic complexity than what you had:

Function GetFoos(a As Whatever) As Foo()
    Return Iterator Function()
               Yield a.First
               Yield a.Last
           End Function().ToArray
End Function

For @svick yes it works fine with recursion (just not mutual recursion, but honestly by that point I think your code has become too complex):

Sub M()
    Dim f As Func(Of Integer, Integer) = Function(n)
                                             Return If(n = 1, 1, n * f(n - 1))
                                         End Function
    Dim x = f(5)
End Sub

For @paulomorgado yes it can infer the return type of the lambda: (the only thing we didn't yet implement is inferring the return of a method which mentions itself, since this problem doesn't have a general solution, but whatever we come up with for local functions could just as well be applied to local functions)

Sub M()
    Dim f = Function(n As Integer)
                Return If(n = 1, 1, n * 2)
            End Function
    Dim x = f(5)
End Sub

For @svick yes VB already allows what amount to async blocks. The only additional syntactic complexity is a set of parentheses at the end:

Async Sub M()
    Dim task1 = Async Function()
                    Console.WriteLine(Await FooAsync())
                End Function()
    Dim task2 = Async Function()
                    Console.WriteLine(Await BarAsync())
                End Function()
    Await Task.WhenAll(task1, task2)
End Sub

What makes all this possible? (1) Lambda expressions have a default type, just like most other expressions in C#. This would be a desirable feature to add to C# regardless. (2) When you define a lambda which refers to itself, the VB compiler knows that any "use before assignment" error would be incorrect, so it suppresses the error in this case. (3) There are iterator lambdas.

I haven't yet seen examples in this thread where the heavyweight addition of an entire new concept would by syntactically any more convenient than the status quo.

If anyone makes the comment that inner functions would be more efficient than using a lambda? -- strongly disagree. It's up to the compiler what to optimize and what kind of IL to emit. In all these cases the compiler's completely at liberty to emit for the lambdas the same IL as it would for inner functions. Actually, in cases where it doesn't do any capture, I reckon it already more or less does.

@agat50
Copy link

agat50 commented May 20, 2015

@svick yes, some kind of static Eval\EvalAsync would help (not very handy cause of verbosity and lambda debug(2015 solve last)). My case is more about reorganizing code, not shorter lambda notation, it's useful in some narrow spaced places like exception filtering. We get aggressive inline code, small temp vars scope. Your case is interesting too, i didn't think much about async blocks.

Simple code replacement (my proposal)

var i = 
{
    await Task.Delay(1000);
    return 3;
};

// equals

int i;
{
    await Task.Delay(1000);
    i = 3;
}

Or actually create anonymous method

var i = await {
    await Task.Delay(1000);
    return 3;
}.ConfigureAwait(false);

@agocke
Copy link
Member

agocke commented May 20, 2015

@paulomorgado Expression bodies will be allowed.

Factorial is an example of a recursive function whose return type could be inferred. However, the general case requires H-M and during the meeting we agreed that we didn't want to do that kind of non-local analysis right now.

@agocke
Copy link
Member

agocke commented May 20, 2015

@ljw1004 You're cheating by using VB syntax ;) An iterator lambda involves an unnecessary delegate and invocation and additional syntax for type annotations in C# . Local functions seem substantially better.

Edit: Rereading, I see you're proposing more features to get around this, like default delegate types and, I assume, implicit delegate conversion in C#. That may provide the benefits of iterator local functions, but I would regard those as out of scope for the current discussion.

@HaloFour
Copy link

Yeah, I think eliminating the delegate invocation would be a great benefit. Being able to enclose the local scope through implicit parameters sounds like it could yield good performance benefits as well.

I do think that there is value for C# to support iterator lambdas. Sometimes you just want to pass an iterator inline to a LINQ method like SelectMany.

Any thoughts to blurring the line between local functions and lambdas? For example, if the delegate instance is never combined, invoked dynamically nor leaves the scope of the method, why not optimize by converting to a local method?

@agocke
Copy link
Member

agocke commented May 20, 2015

@HaloFour I think there's definitely some room for improvement in optimization for lambdas as well.

@orthoxerox
Copy link
Contributor

Is there a reason why a static modifier is not allowed? If the function doesn't close over any variables it should be rewritten as a static function by the compiler anyway; explicitly marking it as static would allow us to declare that we want the compiler to verify we're not accessing object or method state in it.

@HaloFour
Copy link

@orthoxerox I don't think it makes sense to conflate the static modifier as a declaration of what the local function can or can't enclose. The compiler could always emit a static method even if it did close over this members by passing it silently as an argument.

@gafter
Copy link
Member Author

gafter commented May 21, 2015

@paulomorgado

Will expression bodied funtions be allowed?

Read the discussion. It is proposed.

Can't the return type of f be inferred on this case?

Read the discussion. Inferred return type and self-use are allowed but mutually exclusive.

@VSadov
Copy link
Member

VSadov commented May 21, 2015

Is there a specific reason to not allow local extension methods?

@erik-kallen
Copy link

Wouldn't almost all use cases be solved by just assigning a lambda to var? Define this to use an appropriate System.Func delegate type. Example:

int M() {
    var add = (int a, int b) => a + b;
    return add(2, 3);
}

@aluanhaddad
Copy link

Thank you for the update on this proposal. I have wanted local functions in C# for a while and it sounds like the feature design is progressing very nicely.
I agree with @HaloFour that it would be nice if iterator lambdas were supported, but that seems something that would ultimately be a separate feature.

@erik-kallen This can already be done today (without the pleasant type inference):

int M() {
    Func<int, int, int> add = (a, b) => a + b;
    return add(2, 3);
}

I actually use this pattern sometimes, but it has many drawbacks:
It is has a performance penalty, it cannot be generic, it takes a dependency on a BCL type, and it does not allow for recursion without ugly incantations, etc.

@erik-kallen
Copy link

@aluanhaddad I also use the Func<int, int, int> x = (a, b) => a + b pattern today, but I think it's quite ugly (and it requires specifying the return type, which could be inferred).

As for recursion, I think it would absolutely make sense to change the spec so the variable being assigned to can be used inside the lambda expression. I am aware that there is some kind of esoteric assignment that can cause the variable to be used before it is assigned, but that could be fixed by saying that var x = (int a, int b) => x(a - 1, b); is equivalent to Func<int, int, int> x = null; x = (a, b) => x(a - 1, b).

As for performance, if this is really a concern, the compiler could notice that the delegate can never escape the function and generate a local method; we don't really need additional syntax.

@MgSam
Copy link

MgSam commented May 21, 2015

While I agree with Lucian that a combination of features would make lambdas able to cover a lot of the use cases here, I think local functions are useful regardless because they'll support a lot more than lambdas can; namely generics, default and optional parameters, and params.

At the end of the day, I think whatever solution is ultimately designed should fulfill the use cases illustrated in the proposed spec.

@HaloFour
Copy link

Noodle bakin' time:

How about local functions within lambda bodies? 😉

@gafter
Copy link
Member Author

gafter commented May 24, 2015

@HaloFour Yes, as you can see from the original issue

[X] May use {} or => syntax for function body

@HaloFour
Copy link

@bondsbw That doesn't resolve the scope issue, which is frankly the significantly bigger problem.

Aside that, reading the original proposal above it appears that there is no intention of supporting static local functions. Whether or not they would be emitted as static would be an implementation detail. The proposal also specifically covers enclosing the local scope. It would have to in order to support enclosing the parameters anyway, and any local function or closure can access any identifier declared before it.

At best you'd need a requirement that the local function(s) are the first thing to appear in the constructor body. That would require some specific exceptions to the C# language to permit referencing an identifier before it has been defined, but that would at least prevent the problem of enclosing state that does not yet exist. But I just really don't see this use case being covered by local functions.

@JesOb
Copy link

JesOb commented Aug 20, 2015

Just one suggestion

Allow to write function not only before it usage, but allow just declare it in inner scope so it not be visible from outside.

private Single SomeBigFunction( ... )
{
    //Some big logic using inline finctions

    Single Calc1( Single param1, Single apram2 ){...}
    Single Calc2( Single param1 ){...}
    String Convert( Single[] data, String pattern ){...}
}

We always put public Methods first and private (internal implementation) last, so i want be able to put in same order inline functions.

Just syntax sugar to convert from this:

private Single SomeBigFunction( ... )
{
    //Some big logic using next tree finctions    
}
private    Single Calc1( Single param1, Single apram2 ){...}
private    Single Calc2( Single param1 ){...}
private    String Convert( Single[] data, String pattern ){...}

to this:

private Single SomeBigFunction( ... )
{
    //Some big logic using inline finctions

    Single Calc1( Single param1, Single apram2 ){...}
    Single Calc2( Single param1 ){...}
    String Convert( Single[] data, String pattern ){...}
}

@HaloFour
Copy link

@Jes28

I don't like the idea of applying "hoisting" to local functions in C#. It is considered bad practice in JavaScript (on the list of "Bad Parts" in Douglas Crockford's book). This is especially true given that local functions are to enclose the local scope of the "parent" function. Where the function is defined will matter.

@aluanhaddad
Copy link

@Jes28 the proposal states that local variables will be captured as they are by lambda expressions / anonymous functions. Allowing the declaration order to change would prevent this making the feature significantly less useful and powerful. Anyway everything declared in a method body is private, scoped to an invocation.

@gafter
Copy link
Member Author

gafter commented Apr 17, 2016

@agocke There is a checklist here for local functions that you might find useful.

@GSPP
Copy link

GSPP commented Jul 2, 2016

It would be nice if var parameters were allowed. It would be handy to be able to process anonymous types from LINQ queries e.g.

var myQuery = ... select new { X = 1 };

void ProcessItem(var item, string mode) { ... }

foreach (var item in myQuery) {
 if (...)
  ProcessItem(item, "mode1");
 else
  ProcessItem(item, "mode2");
}

This code is contrived to demonstrate that a local functions with var parameters would be very useful. The function has two callsites. It's not possible to write the computation inline in the for loop without 2x duplication.

var parameters are not normally allowed but since local functions are not exposed to the outside world it seems this could be done more easily here. The same reasoning applies to var returns.

@svick
Copy link
Contributor

svick commented Jul 2, 2016

@GSPP Couldn't you use a tuple instead of an anonymous type? That way, you wouldn't need var.

@aluanhaddad
Copy link

@GSPP if that scenario doesn't work there's a big problem. I'm not sure that var parameters would solve this issue but something should definitely be done to address it.

@svick that's true, but local function should have access to the information in their enclosing scope, that should include type information. Otherwise they become less valuable.

I'm not sure how this can be expressed however. Var parameters don't seem like they would make any difference. Something like decltype would be needed.

@mcintyre321
Copy link

mcintyre321 commented Jan 10, 2017

The hoisting is very weird to c# thinking:
localfunction
I feel like in c#, code using a particular variable shouldn't be callable until after the variable is defined.

@HaloFour
Copy link

@mcintyre321

The local function isn't a variable. It's more like a member scoped to the function.

@khyperia
Copy link
Contributor

@mcintyre321 This is a known bug that has been fixed: #15298

@mcintyre321
Copy link

thanks! Linqpad must be using a not-quite-bleeding-edge compiler!

@gulshan
Copy link

gulshan commented Jan 30, 2017

Does type inference works for local functions return type now? If yes, an example please?

@bbarry
Copy link

bbarry commented Jan 30, 2017

@gulshan I don't believe it does. It was deemed a non-goal some time ago to have both forward references (#5173) and have var for return types (#5168) due to the complexity of implementing both. Perhaps this will be revisited in the future.

tryroslyn

@jaredpar
Copy link
Member

@gulshan @bbarry for C# 7.0 there will be no inference for local function return types. It's a complex feature as @bbarry mentions. It will be considered for future versions of C#.

One potential approach is to take a page from C++. Only allow inference in very specific cases:

  • Single line local functions
  • Cases where all returns have same type

@ghost
Copy link

ghost commented Feb 18, 2018

I suggest another syntax to local functions to be unnested dotnet/csharplang#1329

@ghost ghost unassigned khyperia Feb 18, 2018
@CharlesTaylor7
Copy link

CharlesTaylor7 commented Mar 21, 2018

Can we allow attributes on local functions?
Pretty please?

@svick
Copy link
Contributor

svick commented Mar 21, 2018

@CharlesTaylor7 There's an issue for that: dotnet/csharplang#794.

@agocke
Copy link
Member

agocke commented Mar 21, 2018

Yes, @svick is right. Also, all language discussion should go on https://github.com/dotnet/csharplang. I'm going to lock this discussion to prevent misfiling.

@dotnet dotnet locked as resolved and limited conversation to collaborators Mar 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests