Fix extra warning for unused local function

[Youssef1313]

Fixes #47463

[Youssef1313]

Since node is actually a BoundCall, then we already know the local function is used. We want to visit it regardless it's conditionally omitted or not.

[jcouv]

I worry that this change may be unsound (allowing accessing an unassigned variable). Please add a test for:

public class C 
{
    public void M() 
    {
        int i;
        local();
        i.ToString();
        
        [Conditional("DEBUG")]
        void local()
        {
           i = 1;   
        }
    }
}

[RikkiGibson]

It will be good to add this test. I believe it will produce the desired error on i.ToString() because we reset the definite assignment state after visiting the call when callsAreOmitted is true.

[RikkiGibson]

Oh wait. I forgot that we disallow non-static local functions from having ConditionalAttribute at all. We have specified that a local function must be static in order to be conditional.

[Youssef1313]

we disallow non-static local functions from having ConditionalAttribute at all.

That saved some extra debugging 😄

[RikkiGibson]

@jcouv Please take another look at your convenience. I do not think there is a supported scenario involving capturing variables in a conditional local function.

[jcouv]

Sounds good. That's great.
Can we update the test (LocalFunction_ConditionalAttributeDisallowed). We should be able to remove the #pragma warning.

[Youssef1313]

CI is failing due to the following test

roslyn/src/Compilers/CSharp/Test/Emit/CodeGen/CodeGenLocalFunctionTests.cs

Lines 5513 to 5540 in 1ed3ef5

	[Fact]
	public void StaticLocalFunction_ConditionalAttribute_Unreferenced()
	{
	var source = @"
	using System.Diagnostics;
	using System;
	class C
	{
	static void Main()
	{
	local1();
	[Conditional(""DEBUG"")]
	static void local1()
	{
	Console.Write(""hello"");
	}
	}
	}
	";
	CreateCompilation(source, parseOptions: TestOptions.Regular9).VerifyDiagnostics(
	// (12,21): warning CS8321: The local function 'local1' is declared but never used
	// static void local1()
	Diagnostic(ErrorCode.WRN_UnreferencedLocalFunction, "local1").WithArguments("local1").WithLocation(12, 21));
	CreateCompilation(source, parseOptions: TestOptions.Regular9.WithPreprocessorSymbols("DEBUG")).VerifyDiagnostics();
	}

Should I delete my test and update the expected for the failing test?

[RikkiGibson]

@Youssef1313 I went in and made the edits to the tests. Thanks for tracking down the root cause of this 😄

I feel confident we want to make the behavior change given by this PR, since it brings us into alignment with how unused local variable warnings work:

using System.Diagnostics;

int x = 42;
local(x); // comment out this call and warning CS0219 is introduced, regardless of build configuration

[Conditional("DEBUG")]
static void local(int x1) { }

[RikkiGibson]

@dotnet/roslyn-compiler Please review this very straightforward bug fix

[jcouv]

LGTM Thanks (iteration 3). Consider updating existing test for [Conditional] on non-static local function.

[Youssef1313]

@jcouv The test you're referring to should fail if we removed the #pragma warning because the local function there is indeed unused. The PR removes the warning only if the method is actually used.

[333fred]

Thanks @Youssef1313!