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.

Sign up for GitHub

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

Fix error on out param of extern local function #49860

Merged
merged 2 commits into from
Dec 10, 2020
Merged

Fix error on out param of extern local function #49860

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f0267f7
Fix error on out param of extern local function
RikkiGibson Dec 9, 2020
d5cbd85
Extern local function assigns arg to out param
RikkiGibson Dec 9, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions src/Compilers/CSharp/Portable/FlowAnalysis/AbstractFlowPass_LocalFunctions.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ protected TLocalFunctionState GetOrCreateLocalFuncUsages(LocalFunctionSymbol loc

public override BoundNode? VisitLocalFunctionStatement(BoundLocalFunctionStatement localFunc)
{
if (localFunc.Symbol.IsExtern)
Copy link
Contributor

@AlekseyTs AlekseyTs Dec 9, 2020 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

localFunc.Symbol.IsExtern [](start = 16, length = 25)

Does this match the criteria used to skip analysis for type members (non-local functions)? Would it make sense to check for presence of the body instead? #Closed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I found that for member methods, we decide whether to analyze based on whether BindMethodBody returned a non-null body. extern member methods will force a null return from BindMethodBody, which makes us skip analysis even in an error scenario where an extern method has a body.

I didn't want to modify the conditions when a BoundLocalFunctionStatement is given a null body, in order to reduce churn, and felt that this check resulted in useful diagnostic behaviors (i.e. if the function is required to have a body but one is missing, we might as well also let you know about things that are required to be included in that body), which are consistent enough with the diagnostics given for member methods.

{
// an extern local function is not permitted to have a body and thus shouldn't be flow analyzed
return null;
}

var oldSymbol = this.CurrentSymbol;
var localFuncSymbol = localFunc.Symbol;
this.CurrentSymbol = localFuncSymbol;
Expand Down
66 changes: 66 additions & 0 deletions src/Compilers/CSharp/Test/Semantic/Semantics/LocalFunctionTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4285,6 +4285,72 @@ int TakeOutParam2(out int x)
);
}

[Fact]
[WorkItem(49500, "https://github.com/dotnet/roslyn/issues/49500")]
public void OutParam_Extern_01()
{
var src = @"
using System.Runtime.InteropServices;

class C
{
void M()
{
int x;
local(out x);
x.ToString();

[DllImport(""a"")]
static extern void local(out int x);
}

[DllImport(""a"")]
static extern void Method(out int x);
}";
VerifyDiagnostics(src);
}

[Fact]
[WorkItem(49500, "https://github.com/dotnet/roslyn/issues/49500")]
public void OutParam_Extern_02()
{
var src = @"
using System.Runtime.InteropServices;

class C
{
void M()
{
local1(out _);
local2(out _);
local3(out _);

[DllImport(""a"")]
static extern void local1(out int x) { } // 1

static void local2(out int x) { } // 2

static void local3(out int x); // 3, 4
}

[DllImport(""a"")]
static extern void Method(out int x);
}";
VerifyDiagnostics(src,
// (13,28): error CS0179: 'local1(out int)' cannot be extern and declare a body
// static extern void local1(out int x) { } // 1
Diagnostic(ErrorCode.ERR_ExternHasBody, "local1").WithArguments("local1(out int)").WithLocation(13, 28),
// (15,21): error CS0177: The out parameter 'x' must be assigned to before control leaves the current method
// static void local2(out int x) { } // 2
Diagnostic(ErrorCode.ERR_ParamUnassigned, "local2").WithArguments("x").WithLocation(15, 21),
// (17,21): error CS8112: Local function 'local3(out int)' must declare a body because it is not marked 'static extern'.
// static void local3(out int x); // 3, 4
Diagnostic(ErrorCode.ERR_LocalFunctionMissingBody, "local3").WithArguments("local3(out int)").WithLocation(17, 21),
// (17,21): error CS0177: The out parameter 'x' must be assigned to before control leaves the current method
// static void local3(out int x); // 3, 4
Diagnostic(ErrorCode.ERR_ParamUnassigned, "local3").WithArguments("x").WithLocation(17, 21));
}

[Fact]
[WorkItem(13172, "https://github.com/dotnet/roslyn/issues/13172")]
public void InheritUnsafeContext()
Expand Down