roslyn icon indicating copy to clipboard operation
roslyn copied to clipboard

Bug: Conditional Operator stores ref result in a non-ref-temporary (Only when SpillSequenceSpiller is used - e.g. for async/await)

Open bernd5 opened this issue 1 year ago • 2 comments

Version Used: 847d588

Steps to Reproduce:

Compile and execute the following code:

using System;
using System.Threading.Tasks;
_ = Foo.NumberBuggy().Result;
Console.WriteLine("--------");
_ = Foo.NumberNotBuggy();

public class Foo{
     public static async Task<int> NumberBuggy()
     {
        string str = "a2";
        int x = 1;
        int y = 2;

        ref int r =
              ref await EvalAsync(str, 1) is "a1" ? ref x
            : ref await EvalAsync(str, 2) is "a2" ? ref y
            : ref System.Runtime.CompilerServices.Unsafe.NullRef<int>();
        
         r++;
         r++;
         r++;
         int xxx = r;
         System.Console.WriteLine(xxx);
         System.Console.WriteLine(x);
         System.Console.WriteLine(y); //should be 5 now!
         return xxx;
    }
    
    public static ValueTask<int> NumberNotBuggy()
     {
        string str = "a2";
        int x = 1;
        int y = 2;

        ref int r =
              ref EvalAsync(str, 1).Result is "a1" ? ref x
            : ref EvalAsync(str, 2).Result is "a2" ? ref y
            : ref System.Runtime.CompilerServices.Unsafe.NullRef<int>();
        
         r++;
         r++;
         r++;
         int xxx = r;
         System.Console.WriteLine(xxx);
         System.Console.WriteLine(x);
         System.Console.WriteLine(y);
         return new(xxx);
    }
    static ValueTask<string> EvalAsync(string s, int i)
    {
        System.Console.WriteLine($"{s} {i}");
        return ValueTask.FromResult(s);
    }
}

Expected Behavior: Same output for both methods (they are not really async)

Actual Behavior: The result of the conditional expression is stored in a non-ref temp local which causes that the later increment operations do not change the original value - just the temp.

Error cause The error is caused in SpillSequenceSpiller

The code should be:

var tmp = _F.SynthesizedLocal(node.Type, kind: SynthesizedLocalKind.Spill, syntax: _F.Syntax,
   refKind: node.IsRef ? RefKind.Ref : RefKind.None);

But how can we determine if the result should be ref-readonly or not?

bernd5 avatar Jun 23 '24 12:06 bernd5

A simpler version (makes use of spilling with general is pattern):

string str = "a2";
int x = 1;
int y = 2;

ref int r =
      ref str is "Hallo" ? ref x
    : ref str is { Length: >= 2 and <= 10 or 22 } ? ref y
    : ref System.Runtime.CompilerServices.Unsafe.NullRef<int>();

r++;
r++;
r++;
int xxx = r;
System.Console.WriteLine(xxx); //5
System.Console.WriteLine(x); //1
System.Console.WriteLine(y); //expected 5 - but we get 2

bernd5 avatar Jun 23 '24 13:06 bernd5

And a version with ref readonly:

using System;


string str = "a2";
int x = 1;
int y = 2;

ref readonly var xx = ref x; 

ref readonly int r =
      ref Eval(str, 1) is "Hallo" ? ref xx
    : ref Eval(str, 2) is { Length: >= 2 and <= 10 or 22 } ? ref y
    : ref System.Runtime.CompilerServices.Unsafe.NullRef<int>();

ref var rx = ref System.Runtime.CompilerServices.Unsafe.AsRef(in r);

rx++;
rx++;
rx++;
int xxx = r;
System.Console.WriteLine(xxx);
System.Console.WriteLine(x);
System.Console.WriteLine(y);


static T Eval<T>(T s, int i)
{
    System.Console.WriteLine($"{s} {i}");
    return s;
}

bernd5 avatar Jun 23 '24 13:06 bernd5