Roslyn: Definite assignment errors are sometimes reported in unreachable code in local functions

[agocke]

The following program:

        static void Main(string[] args)
        {
            void Local1()
            {
                int x;
                Console.WriteLine(x); // error
            }

            void Local2()
            {
                int x;
                Action f = () =>
                {
                    Console.WriteLine(x); // error
                };
            }

            void Local3()
            {
                void Local()
                {
                    int x;
                    Console.WriteLine(x); // error
                }
            }

            void Local4()
            {
                void Local()
                {
                    Action f = () =>
                    {
                        int x;
                        Console.WriteLine(x); // OK
                    };
                }
                Local(); // OK
            }

            void Local5()
            {
                Action f = () =>
                {
                    int x;
                    Console.WriteLine(x); // OK
                };
            }
        }

should have no errors reported since all of those local functions have no calls and are thus unreachable.

@gafter To confirm I'm reading the spec correctly on this.

[TessenR]

@agocke Why are there different rules for unused local functions and unused private methods? Both aren't accessed directly. Both can be accessed via reflection. Somehow you decided that local functions are less 'reachable'. Could you please clarify the reasoning here?

As a user of the feature I'd probably type a local function first and then use it somewhere in the method (probably not in the same place where I'd declare it). It would be sad if obvious errors weren't reported until after I used it.

[agocke]

@Tessenr

1. Reachability is only defined in the language for intra-method analysis. Specifically, the following statement of the spec (§ 8.1) holds:

   The block of a function member is always considered reachable. By successively evaluating the reachability rules of each statement in a block, the reachability of any given statement can be determined.

2. All compiler generated methods with unspeakable names are considered implementation details and we regularly change how they are emitted. Dependence via reflection is considered unsupported.

[gafter]

@agocke The definite assignment section of the spec is MIA, but I expect it will say something like this:

The body of a statement-bodied lambda expression or local function is considered reachable.

That is consistent with the way we already treat lambdas in unreachable code. Try this, for example:

c# public static void Main() { return; Func<int> a = () => { int result; return result; // error }; }

However, it would be reasonable to handle local functions differently because we track the callers and because it would be useful to programmers. At this point that would be a feature request, as C# 7 is already in the bag. Please propose it in the csharplang repo.

[agocke]

@gafter Then I don't think this issue is resolved -- either the behavior of the lambdas in the local function is a bug since definite assignment errors should be reported or the behavior of the statements in the local function are incorrect since no errors should be reported.

There's definitely a bug here but we have to decide which way we want to go on blocks for local functions. Personally I think blocks in local functions should be treated as unreachable if there is not a reachable call or conversion.

[agocke]

Also, if this statement is accurate:

The body of a statement-bodied lambda expression is considered reachable.

Then we have a bug no matter what. The unreachability of local functions wouldn't affect lambda reachability.

[agocke]

This also opens up a new spec hole since we currently have no concept of what assignment context a local function is in outside of the context in which it is called.

Do we use the declaration point of the local function to decide assignment if and only if there are no calls to the local function?

[gafter]

Very good points.

[agocke]

@gafter Actually I'm going to go further and say that the current behavior for lambdas is also incorrect. The C# 5 spec states in § 7.15:

A block body of an anonymous function is reachable (§8.1) unless the anonymous function occurs inside an unreachable statement.

In my opinion we should not be reporting definite assignment errors for lambdas in unreachable statements.

[agocke]

Assigning back to me because I think there are unresolved language design issues and I have a proposal in mind to address them.

[jcouv]

Updating milestone back to 15.6, since this was fixed in 15.6 (https://github.com/dotnet/roslyn/pull/23749)