In some cases "ref" is less functional than "in"/"ref readonly"

[KorneiDontsov]

I ran into a difference in functionality of "ref" and "in"/"ref readonly" that looks awkward to me.

static class Program {
    struct Subject { }

    static ref readonly Subject InvokeReadOnlyRefExtension (this in Subject subject) =>
        ref subject;

    static ref Subject InvokeRefExtension (this ref Subject subject) =>
        ref subject;

    static void Main () {
        // Legal
        new Subject().InvokeReadOnlyRefExtension();

        // ERROR: 'ref this' argument must be an assignable variable, field or an array element.
        new Subject().InvokeRefExtension();
    }
}

I believe I understand why this error exists, but since compiler does a bit of magic for the "in" parameters, it could do the same for the "ref" parameters, couldn't it?

[YairHalberstadt]

The most common reason to use a ref parameter, is to reassign it. In this case the reassignment would be invisible, so the use is probably incorrect. Hence why it's not worth doing the same trick as for in.

[KorneiDontsov]

@YairHalberstadt, I agree. But it's not the only reason to use ref. Another reason is to change field(s) of mutable struct (in other words, manipulate structure as an object on stack). This helps to use struct instead of class to not allocate object in heap. A good example may be the pattern "Builder".

interface IEventLogger {
    ValueTask LogAsync
        (String eventName,
         IEnumerable<(String name, Object? value)> args,
         DateTimeOffset timestamp);
}

class EventLogBuilder {
    readonly IEventLogger eventLogger;

    readonly String eventName;

    List<(String name, Object? value)>? args;

    DateTimeOffset? timestamp;

    public EventLogBuilder (IEventLogger eventLogger, String eventName) {
        this.eventLogger = eventLogger;
        this.eventName = eventName;
    }

    public EventLogBuilder With (String name, Object? value) {
        args ??= new List<(String, Object?)>(capacity: 4);
        args.Add((name, value));
        return this;
    }

    public EventLogBuilder WithTimestamp (DateTimeOffset timestamp) {
        this.timestamp = timestamp;
        return this;
    }

    public ValueTask Async () =>
        eventLogger.LogAsync(
            eventName,
            args ?? Enumerable.Empty<(String, Object?)>(),
            timestamp ?? DateTimeOffset.Now);
}

static class EventLoggerFunctions {
    public static EventLogBuilder Log (this IEventLogger eventLogger, String eventName) =>
        new EventLogBuilder(eventLogger, eventName);
}

static class ExampleModule {
    public static async void Example (IEventLogger eventLogger) =>
        await eventLogger.Log("example-is-presented")
            .With("builder-is", "class")
            .Async();
}

In this example I use class to implement builder. So I allocate once again to write fluent code. This could be avoided with struct if it wouldn't have the restriction that we are discussing.

interface IEventLogger {
    ValueTask LogAsync
        (String eventName,
         IEnumerable<(String name, Object? value)> args,
         DateTimeOffset timestamp);
}

struct EventLogBuilder {
    public readonly IEventLogger eventLogger;

    public readonly String eventName;

    public List<(String name, Object? value)>? args;

    public DateTimeOffset? timestamp;

    public EventLogBuilder (IEventLogger eventLogger, String eventName) {
        this.eventLogger = eventLogger;
        this.eventName = eventName;
        args = null;
        timestamp = null;
    }
}

static class EventLoggerFunctions {
    public static EventLogBuilder Log (this IEventLogger eventLogger, String eventName) =>
        new EventLogBuilder(eventLogger, eventName);

    public static ref EventLogBuilder With (this ref EventLogBuilder builder, String name, Object? value) {
        builder.args ??= new List<(String, Object?)>(capacity: 4);
        builder.args.Add((name, value));
        return ref builder;
    }

    public static ref EventLogBuilder WithTimestamp (this ref EventLogBuilder builder, DateTimeOffset timestamp) {
        builder.timestamp = timestamp;
        return ref builder;
    }

    public static ValueTask Async (this ref EventLogBuilder builder) =>
        builder.eventLogger.LogAsync(
            builder.eventName,
            builder.args ?? Enumerable.Empty<(String, Object?)>(),
            builder.timestamp ?? DateTimeOffset.Now);
}

static class ExampleModule {
    public static async void Example (IEventLogger eventLogger) =>
        // ERROR: 'ref this' argument must be an assignable variable, field or an array element.
        await eventLogger.Log("example-is-presented")
            .With("builder-is", "struct")
            .Async();
}

Of course, I can just copy structure in every method instead of using ref, but real examples contains a lot of method invocations like With("name", value) and more parameter types such as cancellation token, timeout, ect, in which cases using struct with copying is slower than allocating object in memory.

I propose to remove that error and set behaviour that in parameters already have, because:

• There are the real cases where the error blocks to write code productive and handsome at the same time;
• The smaller the difference between similar instructions, the easier language is to understand and use. The error came as a surprise to me because I already used the pattern I showed with in and ref readonly and I didn't even suspect there can be such difference.
• The error does not have reasons to exist except "protection of fool". Instead compiler could produce a warning that struct value is initialized but isn't used (which is typical warning for general-purpose languages, I think).

[YairHalberstadt]

in which cases using struct with copying is slower than allocating object in memory.

Can you run some benchmarks showing this is the case.

Also why not make the builder methods instance methods?

[KorneiDontsov]

Also why not make the builder methods instance methods?

I cannot return ref this. And return this means copying which is not productive.

Can you run some benchmarks showing this is the case.

I quickly wrote the benchmarks and pubish them on KorneiDontsov/CSharpLangIssue3418Benchmarks.
I got results:

BenchmarkDotNet=v0.12.1, OS=Windows 10.0.18363.778 (1909/November2018Update/19H2)
Intel Core i7-8750H CPU 2.20GHz (Coffee Lake), 1 CPU, 12 logical and 6 physical cores
.NET Core SDK=3.1.102
  [Host]     : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT
  DefaultJob : .NET Core 3.1.2 (CoreCLR 4.700.20.6602, CoreFX 4.700.20.6702), X64 RyuJIT

Method	Mean	Error	StdDev	Gen 0	Gen 1	Gen 2	Allocated
Object	208.5 ns	0.52 ns	0.48 ns	0.1342	-	-	632 B
CopyingStruct	317.9 ns	1.68 ns	1.49 ns	0.1173	-	-	552 B
ReferencedStruct	218.9 ns	0.62 ns	0.55 ns	0.1173	-	-	552 B

[YairHalberstadt]

There's not a huge difference between them. It's highly unlikely that this will be a bottleneck in an application. If it is, you can always assign the builder to a local.

It seems to me like the risk of creating a pit of failure, is greater than the potential benefits here.

[KorneiDontsov]

@YairHalberstadt, to me it is not. I have no more arguments. Thank you for your time. Feel free to close the issue.

[YairHalberstadt]

@KorneiDontsov
I don't work for Microsoft, just somebody who lurks around here :-). It's not my decision to make and perhaps the language team will disagree with me.

[KorneiDontsov]

@YairHalberstadt, sorry, I confused "Contributor" with "Collaborator" :) I'm not good at English

[jnm2]

The most common reason to use a ref parameter, is to reassign it. In this case the reassignment would be invisible, so the use is probably incorrect. Hence why it's not worth doing the same trick as for in.

In the case of value types, any instance method call can reassign this. Extension methods on refs of value types would have the same semantics.