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

Incorrect rewrite using foreach variable as ref parameter #20

Open
jgardiol opened this issue Jan 19, 2018 · 1 comment
Open

Incorrect rewrite using foreach variable as ref parameter #20

jgardiol opened this issue Jan 19, 2018 · 1 comment

Comments

@jgardiol
Copy link

I tried running roslyn-linq-rewrite (built from source) on a big solution and found some issues. There's an issue with expressions capturing the foreach loop variable when it is a struct type (eg. when iterating over an IDictionary<K,V>, the loop variable is of type KeyValuePair<K,V> which is a struct)

I made a minimal code snippet reproducing the issue:

public void Bugged(IDictionary<string, string> dict, IList<string> list) {
    foreach (KeyValuePair<string, string> kv in dict) {
        if (list.Any(item => item == kv.Value)) {
            list.Remove(kv.Value);
        }
    }
}

This gets rewritten to

public void Bugged(IDictionary<string, string> dict, IList<string> list)
{
    foreach (KeyValuePair<string, string> kv in dict)
    {
        if (Bugged_ProceduralLinq1(list, ref kv))
        {
            list.Remove(kv.Value);
        }
    }
}

bool Bugged_ProceduralLinq1(System.Collections.Generic.IList<string> _linqitems, ref System.Collections.Generic.KeyValuePair<string, string> kv)
{
    if (_linqitems == null)
        throw new System.ArgumentNullException();
    foreach (var _linqitem in _linqitems)
    {
        if (_linqitem == kv.Value)
        {
            return true;
        }
    }

    return false;
}

Which produces a compilation error (CS1567)

@bernd5
Copy link

bernd5 commented Jun 21, 2019

The problem is, that a foreach variable is "readonly". The subroutine is not allowed to change the struct.
So we must submit the reference via readonly ref which is called "in" now in CSharp 7.

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

No branches or pull requests

2 participants