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

Pass ByRef parameters of parent into hoisted function by ref #1126

Conversation

TymurGubayev
Copy link
Contributor

Problem

Input:

Sub S(ByRef x As Integer)
    Dim i As Integer = 0
    If F1(x, i) Then
    End If
End Sub

Function F1(x As Integer, ByRef o As Object) As Boolean : End Function

Erroneous output:

public void S(ref int x)
{
    int i = 0;
    //CS1628: Cannot use ref, out, or in parameter 'x' inside an anonymous method, lambda expression, query expression, or local function
    bool localF1() { object argo = i; var ret = F1(x, ref argo); i = Conversions.ToInteger(argo); return ret; }

    if (localF1())
    {
    }
}

public bool F1(int x, ref object o)
{
    return default;
}

Solution

  • Any comments on the approach taken, its consistency with surrounding code, etc.
    Analyze arguments in HoistAndCallLocalFunctionAsync, if any are ByRef parameters, create an ArgumentList an a ParameterList to use in the local function we are about to create.
  • Which part of this PR is most in need of attention/improvement?
    Currently, I'm passing null for the ParameterList from CommonConversions.cs/ConvertEqualsValueClauseSyntaxAsync() - so there is no change in behavior. It might be worth analyzing what happens there and maybe make a similar adjustment (I haven't spent much time on that).
  • At least one test covering the code changed

@GrahamTheCoder GrahamTheCoder merged commit 9e929e1 into icsharpcode:master Jul 28, 2024
2 checks passed
@TymurGubayev TymurGubayev deleted the fix/HoistedOutParameterLambdaUsingByRefParameter/1 branch July 29, 2024 08:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants