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

Bug fix extract local function errors C#7 and below #55446

Merged
merged 8 commits into from
Aug 20, 2021

Conversation

akhera99
Copy link
Member

@akhera99 akhera99 commented Aug 5, 2021

Fixes: #55031

Opts to capture variables instead of creating new parameters in language versions below c#8 because they do not support static local functions.

  • Need to fix the call to the new method

@akhera99 akhera99 requested a review from a team as a code owner August 5, 2021 21:48
Copy link
Contributor

@MaStr11 MaStr11 left a comment

Choose a reason for hiding this comment

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

I started to work on this in #55135 and the proposed solution was to capture or to rename the parameters.

#55135 (comment)

@MaStr11
Copy link
Contributor

MaStr11 commented Aug 16, 2021

@akhera99 If you struggle to find a way to identify a linq range variable: I would suggest adding a property to VariableSymbol:

protected abstract class VariableSymbol
{
    public abstract bool CanBeCapturedByLocalFunction { get; }
}

This should return false in QueryVariableSymbol and true in any other case.

You can expose CanBeCapturedByLocalFunction like so in VariableInfo:

protected class VariableInfo
{
            /// <summary>
            /// Returns true, if the variable could be either passed as a parameter
            /// to the new local function or the local function can capture the variable.
            /// </summary>
            public bool CanBeCapturedByLocalFunction
                => _variableSymbol.CanBeCapturedByLocalFunction;
}

@akhera99
Copy link
Member Author

@akhera99 If you struggle to find a way to identify a linq range variable: I would suggest adding a property to VariableSymbol:

protected abstract class VariableSymbol
{
    public abstract bool CanBeCapturedByLocalFunction { get; }
}

This should return false in QueryVariableSymbol and true in any other case.

You can expose CanBeCapturedByLocalFunction like so in VariableInfo:

protected class VariableInfo
{
            /// <summary>
            /// Returns true, if the variable could be either passed as a parameter
            /// to the new local function or the local function can capture the variable.
            /// </summary>
            public bool CanBeCapturedByLocalFunction
                => _variableSymbol.CanBeCapturedByLocalFunction;
}

Thank you for your help. I was trying to avoid looking explicitly at the AnalyzerResult information, but that is definitely the easiest way to handle it.

@akhera99 akhera99 merged commit 29ba4f8 into dotnet:main Aug 20, 2021
@ghost ghost added this to the Next milestone Aug 20, 2021
@dibarbet dibarbet modified the milestones: Next, 17.0.P4 Aug 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor to create local method creates argument for local const member
4 participants