Readonly fields have an accessible home when accessed from init-only setter

[jcouv]

Fixes #45657

[AlekseyTs]

ConsiderEverything [](start = 96, length = 18)

This option doesn't look right, why differences in nullable annotations, or dynamic, or native int, or tuple names, should make a difference? I think we should ignore all options that could be ignored. #Closed

[AlekseyTs]

I think the problem can be demonstrated with this failing test:

        [Fact]
        public void Test()
        {
            string source = @"
public class C
{
    public static void Main()
    {
        System.Console.WriteLine(C1<int>.F1.content);
        System.Console.WriteLine(C2<int>.F1.content);
    }
}

public struct Container
{
    public int content;
}

class C1<T>
{
    public static readonly Container F1;
    
    static C1()
    {
        C1<T>.F1.content = 2;
    }
}


#nullable enable

class C2<T>
{
    public static readonly Container F1;
    
    static C2()
    {
        C2<T>.F1.content = 2;
    }
}
";
            var comp = CreateCompilation(new[] { source, IsExternalInitTypeDefinition }, parseOptions: TestOptions.RegularPreview, options: TestOptions.DebugExe);
            comp.VerifyEmitDiagnostics();
            CompileAndVerify(comp, expectedOutput: 
@"
2
2
");
        }

---

In reply to: 460448404 [](ancestors = 460448404)

[jcouv]

Thanks much for the test scenario. Added and fixed.

[AlekseyTs]

Done with review pass (iteration 1) #Closed

[AlekseyTs]

verify: Verification.Skipped /* init-only */ [](start = 58, length = 44)

This isn't an init-only scenario, no need to skip verification, I think. #Closed

[jcouv]

I think you're right. I did run into a PE verification error though.

As far as I can tell we're emitting the same IL already and in non-generic scenario too (which also fails PE verification).

Feels like a PE verification bug. Is there a set of IL verification rules that I could reference to confirm this?

            // [ : C::Main][mdToken=0x6000004][offset 0x00000001] Cannot change initonly field outside its .ctor.
            v.VerifyIL("C.Main", @"
{
  // Code size       45 (0x2d)
  .maxstack  1
  IL_0000:  nop
  IL_0001:  ldsflda    ""Container C1<int>.F1""
  IL_0006:  ldfld      ""int Container.content""
  IL_000b:  call       ""void System.Console.Write(int)""
  IL_0010:  nop
  IL_0011:  ldstr      "" ""
  IL_0016:  call       ""void System.Console.Write(string)""
  IL_001b:  nop
  IL_001c:  ldsflda    ""Container C2<int>.F1""
  IL_0021:  ldfld      ""int Container.content""
  IL_0026:  call       ""void System.Console.Write(int)""
  IL_002b:  nop
  IL_002c:  ret
}
");

[AlekseyTs]

LGTM (iteration 2), with minor comment for the test.

[AlekseyTs]

LGTM (iteration 3), assuming CI is passing

[jcouv]

@dotnet/roslyn-compiler for second review. Thanks

[jcouv]

@dotnet/roslyn-compiler for second review. Thanks

[cston]

class in which the field is defined or a variable initializer)) [](start = 131, length = 63)

Is this comment stale?

Same comment in other locations.

[jcouv]

This is an artifact of the GitHub diff view, I think.

Locally, the file shows the correct comment:

                // (9,9): error CS0191: A readonly field cannot be assigned to (except in a constructor or init-only setter of the type in which the field is defined or a variable initializer)
                //         c.field = null; // 1
                Diagnostic(ErrorCode.ERR_AssgReadonly, "c.field").WithLocation(9, 9),