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

Draft spec for local function attributes #3008

Closed
RikkiGibson opened this issue Dec 6, 2019 · 6 comments
Closed

Draft spec for local function attributes #3008

RikkiGibson opened this issue Dec 6, 2019 · 6 comments
Labels

Comments

@RikkiGibson
Copy link
Contributor

RikkiGibson commented Dec 6, 2019

Related to #1888

Attributes

Local function declarations are now permitted to have attributes. Parameters and type parameters on local functions are also allowed to have attributes.

Attributes with a specified meaning when applied to a method, its parameters, or its type parameters will have the same meaning when applied to a local function, its parameters, or its type parameters, respectively.

A local function can be made conditional in the same sense as a conditional method by decorating it with a [ConditionalAttribute]. A conditional local function must also be static.

Extern

The extern modifier is now permitted on local functions. This makes the local function external in the same sense as an external method.

Similarly to an external method, the local-function-body of an external local function must be a semicolon. A semicolon local-function-body is only permitted on an external local function.

An external local function must also be static.

Syntax

The local functions grammar is modified as follows:

local-function-header
    : attributes? local-function-modifiers? return-type identifier type-parameter-list?
        ( formal-parameter-list? ) type-parameter-constraints-clauses
    ;

local-function-modifiers
    : (async | unsafe | static | extern)*
    ;

local-function-body
    : block
    | arrow-expression-body
    | ';'
    ;
@svick
Copy link
Contributor

svick commented Dec 6, 2019

For extern methods that use DllImport, the method name is usually used by the runtime to figure out which function to import. Since the compiler mangles names for local functions, does this mean that extern local functions would be required to explicitly specify DllImportAttribute.EntryPoint?

In other words, would the code in #2740 work as is at runtime?

@HaloFour
Copy link
Contributor

HaloFour commented Dec 6, 2019

@svick

DllImport is a pseudo-attribute, no? I'd think that the compiler would write out the metadata with the original name and not the mangled name. That wouldn't work for custom attributes, though.

@RikkiGibson
Copy link
Contributor Author

When using DllImport on a local function, it is expected that the EntryPoint defaults to the name of the local function as it appears in source.

@svick
Copy link
Contributor

svick commented Dec 6, 2019

@HaloFour Yeah, you're right, I didn't think of that.

@RikkiGibson Good to know.

@RikkiGibson
Copy link
Contributor Author

In the test plan review for this feature, we came up with a scenario where definite assignment safety could be violated in local functions.

using System.Diagnostics;
using System;

class C
{
    void M()
    {
        int x;
        local1();
        Console.WriteLine(x);
 
        [Conditional("DEBUG")]       
        void local1()
        {
            x = 42;
        }
    }
}

In this scenario, x is considered definitely assigned after the call to local1. But if the compilation doesn't define a "DEBUG" symbol, we won't actually emit the call in IL, and we will end up reading uninitialized local variables.

We think we can solve this by adding the requirement that a local function with ConditionalAttribute must be static.

@jcouv
Copy link
Member

jcouv commented Oct 21, 2020

Closing in favor of #1888 (added link to speclet there)

@jcouv jcouv closed this as completed Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants