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

Parameterless struct initialization re-uses values #57870

Closed
jaredpar opened this issue Nov 18, 2021 · 0 comments · Fixed by #57925
Closed

Parameterless struct initialization re-uses values #57870

jaredpar opened this issue Nov 18, 2021 · 0 comments · Fixed by #57925
Assignees
Milestone

Comments

@jaredpar
Copy link
Member

jaredpar commented Nov 18, 2021

Simplified repro from original issue:

using System;

var x = new S3Options { BasePath = "p/a/t/h", Bucket = "root" };
var y = new S3Options { BasePath = "p/a/t/h" };

Console.WriteLine(y.Bucket);

public record struct S3Options
{
    public string? Bucket { get; init; }
    public string? BasePath { get; init; } = "value";
}

This prints out root where it should print out nothing. The reason this is happening is that we are re-using a local for initialization purposes. This is a practice we've had since probably C# 1.0. Prior to C# 10 though we cleared out the local between uses via .initobj instruction. In C# 10 though when there is a parameterless struct ctor, explicit or implicit because of initializers, we call the .ctor on the struct instead.

IL without paramterless ctor

        IL_0024: ldloca.s 2
        IL_0026: initobj S3Options
        IL_002c: ldloca.s 2
        IL_002e: ldstr "p/a/t/h"

IL with parameterless ctor

        IL_0023: ldloca.s 2
        IL_0025: call instance void S3Options::.ctor()
        IL_002a: ldloca.s 2
        IL_002c: ldstr "p/a/t/h"

The .ctor call though does not clear out all of the existing values, it just sets the minimal necessary set. Because Bucket doesn't have an explicit initializer it ends up re-using the previous value.

@jaredpar jaredpar added this to the 17.1 milestone Nov 18, 2021
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Nov 18, 2021
@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Nov 22, 2021
Efruit added a commit to Efruit/RobustToolbox that referenced this issue Nov 26, 2021
Efruit added a commit to Efruit/RobustToolbox that referenced this issue Nov 30, 2021
Efruit added a commit to Efruit/RobustToolbox that referenced this issue Dec 12, 2021
Acruid pushed a commit to space-wizards/RobustToolbox that referenced this issue Dec 12, 2021
* Shared/Utility: Define new FormattedMessage core types

* Shared/Utility: Move MarkupParser to a new namespace, update to new FormattedText

* Shared/Serialization: Temporary fix for FormattedMessageSerializer

* Scripting/ScriptInstanceShared: Move to new FormattedMessage.Builder

* Shared/Utility: Add a FormattedMessage loader to the .Builder

* Server/Scripting: Port SciptHost to FormattedMessage.Builder

* UserInterface/RichTextEntry: NOP out almost everything

not gonna bother fixing it until more groundwork is laid

* Shared/Utility: Expand Utility.Extensions a bit

strictly for pesonal reasons

* Client/UserInterface: Add the base TextLayout engine

* Client/Graphics: Add a Font Library manager

* Graphics/TextLayout: Finish up implementing the TextLayout engine

* Utility/FormattedMessage: Add yet another hack to keep the serializer in service

* Commands/Debug: Use FormattedMessage.Builder

* Console/Completions: Use FormattedMessage.Builder

* Utility/FormattedMessage: Add `AddMessage` methods

* Console/ScriptConsole: Use FormattedMessage.Builder

* Client/Log: Use FormattedMessage.Builder

* CustomControls/DebugConsole: Use FormattedMessage.Builder

* Controls/OutputPanel: Use FormattedMessage.Builder, NOP `Draw` pending rewrite

* Controls/RichTextLabel: Use FormattedMessage.Builder, NOP `Draw` pending rewrite

* UnitTesting: Update FormattedMessage/Markup Tests

They will NOT pass yet, but I don't care; it compiles.

* Utility/FormattedMessage: Fix some off-by-one Builder bugs

* Utility/FormattedMessage: Continue cleanup, test compliance

* Utility/FormattedMessage: Work around dotnet/roslyn#57870

* Utility/FormattedMessage: Move ISectionable from TextLayout, implement it for FormattedMessage

* UserInterface/TextLayout: Add a `postcreate` function to set up new `TIn`s

Apparently Roslyn isn't big-brained enough to understand that a
closure of type `Func<T>` means that `var n = new(); return n;` requires
`new()` to return a `T`.

Ironically, it's significantly less cbt to add this than to convert
that big tuple in `Layout` in to a class or struct of some sort
(to initialize the `List<>`s).

* UserInterface/TextLayout: Throw if `Meta` isn't recognized

TODO warning go brrr

* Graphics/FontLibrary: Add a `DummyVariant`

* UserInterface/UITheme: Move to FontLibraries

* UserInterface/TextLayout: Move to an un-nested `ImmutableArray`

* UserInterface/RichTextEntry: Go ahead. Draw.

* Markup/Basic: Add extension & helpers for FormattedMessage.Builder

* Markup/Basic: Add `EscapeText` back in

A forgotten casualty of the great Markup separation of 2021

* Graphics/FontLibrary: Clean up bit magic, ensure that at least one font is picked

* Graphics/FontLibrary: Add diagnostics to the "no fonts" exception

* UserInterface/TextLayout: Scrap `Word`, return to `Offset`

* UserInterface/TextLayout: A whole bunch of hard-fought bugfixes

* Utility/FormattedMessage: Add a static, empty FormattedMessage

* Utility/FormattedMessage: Fix. Bugs.

* UserInterface/RichTextEntry: Bug fixin'

* UserInterface: CSS teim

* Markup/Basic: Add an optional "default" style to use

* Utility/FormattedMessage: I'm surprised I only made this mistake once.

* Log/DebugConsoleLogHandler: work around lack of a default style
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants