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

Difference in struct initialization causing values to be copied between 2 variables #58771

Closed
teh-hippo opened this issue Jan 11, 2022 · 6 comments

Comments

@teh-hippo
Copy link

Description

The code provided in the reproduction steps fails with the following exception, and I'm not able to explain why. I believe it may be a bug with the framework, but it may be a convention I'm not aware of.

My expectation would be that in all cases provided, the result is the same - a 'new' struct is created / allocated.

0 == subjectB.ValueB
Expected: 0, Actual: 5
   at Program.<<Main>$>g__FailsWithSingleInit|0_0() in C:\repos\Program.cs:line 24
   at Program.<Main>$(String[] args) in C:\repos\Program.cs:line 9

Reproduction Steps

The following code will fail on the final assertation in FailsWithSingleInit.

I have made a slightly modified version on .NET fiddle, throwing an exception instead.
https://dotnetfiddle.net/LQh6dG

using System.Diagnostics;

PassesNormally();
PassesWithStringAdded();
PassesWhenInitAll();
PassesWhenRunAllViaLocalStaticInit();

// This will fail.
FailsWithSingleInit();



static void FailsWithSingleInit()
{
    // Setup 2 items.
    var subjectA = new ExampleWithStringInitOne();
    var subjectB = new ExampleWithStringInitOne { ValueB = 5, ValueA = 10u };
    Debug.Assert(subjectA.ValueA != subjectB.ValueA, "subjectA.ValueA != subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectA.ValueA, subjectB.ValueA);
    Debug.Assert(subjectA.ValueB != subjectB.ValueB, "subjectA.ValueB != subjectB.ValueB", "Expected: {0}, Actual: {1}", subjectA.ValueB, subjectB.ValueB);

    // Re-assign one.
    subjectB = new ExampleWithStringInitOne { ValueA = 10u };
    Debug.Assert(10u == subjectB.ValueA, "10u == subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectB.ValueA, 10u);
    Debug.Assert(0 == subjectB.ValueB, "0 == subjectB.ValueB", "Expected: {0}, Actual: {1}", 0, subjectB.ValueB);
}

static void PassesNormally()
{
    // Setup 2 items.
    var subjectA = new ExampleWithoutString();
    var subjectB = new ExampleWithoutString { ValueB = 5, ValueA = 10u };
    Debug.Assert(subjectA.ValueA != subjectB.ValueA, "subjectA.ValueA != subjectB.ValueA",
        "Expected: {0}, Actual: {1}", subjectA.ValueA, subjectB.ValueA);
    Debug.Assert(subjectA.ValueB != subjectB.ValueB, "subjectA.ValueB != subjectB.ValueB", "Expected: {0}, Actual: {1}",
        subjectA.ValueB, subjectB.ValueB);

    // Re-assign one.
    subjectB = new ExampleWithoutString { ValueA = 10u };
    Debug.Assert(10u == subjectB.ValueA, "10u == subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectB.ValueA,
        10u);
    Debug.Assert(0 == subjectB.ValueB, "0 == subjectB.ValueB", "Expected: {0}, Actual: {1}", 0, subjectB.ValueB);
}

static void PassesWithStringAdded()
{
    // Setup 2 items.
    var subjectA = new ExampleWithString();
    var subjectB = new ExampleWithString { ValueB = 5, ValueA = 10u };
    Debug.Assert(subjectA.ValueA != subjectB.ValueA, "subjectA.ValueA != subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectA.ValueA, subjectB.ValueA);
    Debug.Assert(subjectA.ValueB != subjectB.ValueB, "subjectA.ValueB != subjectB.ValueB", "Expected: {0}, Actual: {1}", subjectA.ValueB, subjectB.ValueB);

    // Re-assign one.
    subjectB = new ExampleWithString { ValueA = 10u };
    Debug.Assert(10u == subjectB.ValueA, "10u == subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectB.ValueA, 10u);
    Debug.Assert(0 == subjectB.ValueB, "0 == subjectB.ValueB", "Expected: {0}, Actual: {1}", 0, subjectB.ValueB);
}

static void PassesWhenInitAll()
{
    // Setup 2 items.
    var subjectA = new ExampleWithStringInitAll();
    var subjectB = new ExampleWithStringInitAll { ValueB = 5, ValueA = 10u };
    Debug.Assert(subjectA.ValueA != subjectB.ValueA, "subjectA.ValueA != subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectA.ValueA, subjectB.ValueA);
    Debug.Assert(subjectA.ValueB != subjectB.ValueB, "subjectA.ValueB != subjectB.ValueB", "Expected: {0}, Actual: {1}", subjectA.ValueB, subjectB.ValueB);

    // Re-assign one.
    subjectB = new ExampleWithStringInitAll { ValueA = 10u };
    Debug.Assert(10u == subjectB.ValueA, "10u == subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectB.ValueA, 10u);
    Debug.Assert(0 == subjectB.ValueB, "0 == subjectB.ValueB", "Expected: {0}, Actual: {1}", 0, subjectB.ValueB);
}

static void PassesWhenRunAllViaLocalStaticInit()
{
    RunTest<ExampleWithoutString>();
    RunTest<ExampleWithString>();
    RunTest<ExampleWithStringInitOne>();

    static void RunTest<T>() where T : IExample, new()
    {
        
        // Setup 2 items.
        var subjectA = new T();
        var subjectB = new T { ValueB = 5, ValueA = 10u };
        Debug.Assert(subjectA.ValueA != subjectB.ValueA, "subjectA.ValueA != subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectA.ValueA, subjectB.ValueA);
        Debug.Assert(subjectA.ValueB != subjectB.ValueB, "subjectA.ValueB != subjectB.ValueB", "Expected: {0}, Actual: {1}", subjectA.ValueB, subjectB.ValueB);

        // Re-assign one.
        subjectB = new T { ValueA = 10u };
        Debug.Assert(10u == subjectB.ValueA, "10u == subjectB.ValueA", "Expected: {0}, Actual: {1}", subjectB.ValueA, 10u);
        Debug.Assert(0 == subjectB.ValueB, "0 == subjectB.ValueB", "Expected: {0}, Actual: {1}", 0, subjectB.ValueB);
    }
}

internal interface IExample
{
    public uint ValueA { get; init; }
    public int ValueB { get; init; }
}

internal struct ExampleWithoutString : IExample
{
    public uint ValueA { get; init; }
    public int ValueB { get; init; }
}

internal struct ExampleWithString : IExample
{
    public uint ValueA { get; init; }
    public int ValueB { get; init; }
    public string ValueC { get; init; }
}

internal struct ExampleWithStringInitOne : IExample
{
    public uint ValueA { get; init; }
    public int ValueB { get; init; }
    public string ValueC { get; init; } = "x";
}

#pragma warning disable CA1805
internal struct ExampleWithStringInitAll : IExample
{
    public uint ValueA { get; init; } = 0;
    public int ValueB { get; init; } = 0;
    public string ValueC { get; init; } = "x";
}
#pragma warning restore CA1805

Expected behavior

All provided test examples pass, with no assertation issues.

Actual behavior

The provided code fails with the following:

0 == subjectB.ValueB
Expected: 0, Actual: 5
   at Program.<<Main>$>g__FailsWithSingleInit|0_0() in C:\repos\Program.cs:line 24
   at Program.<Main>$(String[] args) in C:\repos\Program.cs:line 9

Regression?

I'm not sure.

Known Workarounds

The workaround for this case, is to provide a public constructor myself, where all required to be initialised.

Configuration

.NET 6.0
Windows 10, 21H2, 19044.1415
x64

I don't believe it is specific to this configuration, but I haven't tested it on any other platforms.

Other information

Reviewing the IL code, it seems to generate a different path for the failing static method.
I believe it's something funky in there.

@dotnet-issue-labeler
Copy link

I couldn't figure out the best area label to add to this issue. If you have write-permissions please help me learn by adding exactly one area label.

@EgorBo
Copy link
Member

EgorBo commented Jan 11, 2022

Minimal repro:

using System;

class Prog
{
    static int FailsWithSingleInit()
    {
        Foo foo = new Foo { B = 5, A = 10u };
        foo = new Foo { A = 10u };
        return foo.B; // expected: 0, actual: 5
    }

    static void Main() => Console.WriteLine(FailsWithSingleInit()); // prints 5
}

internal struct Foo
{
    public uint A { get; init; }
    public int B { get; init; }
    public string C { get; init; } = "x";
}

My impression that this a bug in Roslyn/C# (this code doesn't compile on sharplab-main, it says error CS8983: A 'struct' with field initializers must include an explicitly declared constructor.).
But it does compile in .NET 6.0.101 (the latest public release)

  .method private hidebysig static int32
    FailsWithSingleInit() cil managed
  {
    .maxstack 2
    .locals init (
      [0] valuetype Foo V_0,
      [1] valuetype Foo foo
    )

    // [47 5 - 47 24]
    IL_0000: ldloca.s     foo
    IL_0002: call         instance void Foo::.ctor()

    // [48 5 - 48 14]
    IL_0007: ldloca.s     foo
    IL_0009: ldc.i4.5
    IL_000a: call         instance void modreq ([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) Foo::set_B(int32)

    // [49 5 - 49 16]
    IL_000f: ldloca.s     foo
    IL_0011: ldc.i4.s     10 // 0x0a
    IL_0013: call         instance void modreq ([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) Foo::set_A(unsigned int32)
    IL_0018: ldloc.1      // foo
    IL_0019: stloc.0      // V_0

    // [50 5 - 50 20]
    IL_001a: ldloca.s     foo
    IL_001c: call         instance void Foo::.ctor()

    // [51 5 - 51 16]
    IL_0021: ldloca.s     foo
    IL_0023: ldc.i4.s     10 // 0x0a
    IL_0025: call         instance void modreq ([System.Runtime]System.Runtime.CompilerServices.IsExternalInit) Foo::set_A(unsigned int32)

    // [52 5 - 52 18]
    IL_002a: ldloc.1      // foo
    IL_002b: stloc.0      // V_0
    IL_002c: ldloca.s     V_0
    IL_002e: call         instance int32 Foo::get_B()
    IL_0033: ret

  } // end of method Prog::FailsWithSingleInit

^ it calls instance void Foo::.ctor() on the same already initialized local (without initobj).

@EgorBo
Copy link
Member

EgorBo commented Jan 11, 2022

reproduces on the latest daily build of .NET 7.0.
cc @jaredpar @333fred

@EgorBo
Copy link
Member

EgorBo commented Jan 11, 2022

dotnet/csharplang#5552 ?

@stephentoub stephentoub transferred this issue from dotnet/runtime Jan 11, 2022
@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 11, 2022
@SupinePandora43
Copy link

Is Regression term fits here?

@cston
Copy link
Member

cston commented Jan 11, 2022

@mikesena, thanks for reporting this issue and for providing the detailed repro.

This is #57870 and has been fixed in 17.1.

With 17.1, the compiler reports errors on the struct declaration that all fields must be assigned:

(113,17): error CS0843: Auto-implemented property must be fully assigned before control is returned to the caller.
(113,17): error CS0843: Auto-implemented property must be fully assigned before control is returned to the caller.

@jaredpar jaredpar added Bug and removed untriaged Issues and PRs which have not yet been triaged by a lead labels Jan 11, 2022
@jaredpar jaredpar added this to the 17.1 milestone Jan 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants