tannergooding commented 6 years ago

Readonly Instance Methods

[x] Proposed: https://github.com/dotnet/csharplang/blob/master/proposals/csharp-8.0/readonly-instance-members.md
[x] Prototype: Done
[x] Implementation: Done
[ ] Specification: Not Started

Summary

Provide a way to specify individual instance members on a struct do not modify state, in the same way that readonly struct specifies no instance members modify state.

It is worth noting that readonly instance member != pure instance member. A pure instance member guarantees no state will be modified. A readonly instance member only guarantees that instance state will not be modified.

All instance members on a readonly struct could be considered implicitly readonly instance members. Explicit readonly instance members declared on non-readonly structs would behave in the same manner. For example, they would still create hidden copies if you called an instance member (on the current instance or on a field of the instance) which was itself not-readonly.

Motivation

Today, users have the ability to create readonly struct types which the compiler enforces that all fields are readonly (and by extension, that no instance members modify the state). However, there are some scenarios where you have an existing API that exposes accessible fields or that has a mix of mutating and non-mutating members. Under these circumstances, you cannot mark the type as readonly (it would be a breaking change).

This normally doesn't have much impact, except in the case of in parameters. With in parameters for non-readonly structs, the compiler will make a copy of the parameter for each instance member invocation, since it cannot guarantee that the invocation does not modify internal state. This can lead to a multitude of copies and worse overall performance than if you had just passed the struct directly by value. For an example, see this code on sharplab

Some other scenarios where hidden copies can occur include static readonly fields and literals. If they are supported in the future, blittable constants would end up in the same boat; that is they all currently necessitate a full copy (on instance member invocation) if the struct is not marked readonly.

Design

Allow a user to specify that an instance member is, itself, readonly and does not modify the state of the instance (with all the appropriate verification done by the compiler, of course). For example:

public struct Vector2
{
    public float x;
    public float y;

    public readonly float GetLengthReadonly()
    {
        return MathF.Sqrt(LengthSquared);
    }

    public float GetLength()
    {
        return MathF.Sqrt(LengthSquared);
    }

    public readonly float GetLengthIllegal()
    {
        var tmp = MathF.Sqrt(LengthSquared);

        x = tmp;    // Compiler error, cannot write x
        y = tmp;    // Compiler error, cannot write y

        return tmp;
    }

    public float LengthSquared
    {
        readonly get
        {
            return (x * x) +
                   (y * y);
        }
    }
}

public static class MyClass
{
    public static float ExistingBehavior(in Vector2 vector)
    {
        // This code causes a hidden copy, the compiler effectively emits:
        //    var tmpVector = vector;
        //    return tmpVector.GetLength();
        //
        // This is done because the compiler doesn't know that `GetLength()`
        // won't mutate `vector`.

        return vector.GetLength();
    }

    public static float ReadonlyBehavior(in Vector2 vector)
    {
        // This code is emitted exactly as listed. There are no hidden
        // copies as the `readonly` modifier indicates that the method
        // won't mutate `vector`.

        return vector.GetLengthReadonly();
    }
}

Readonly can be applied to property accessors to indicate that this will not be mutated in the accessor.

public int Prop1
{
    readonly get
    {
        return this._store["Prop1"];
    }
    readonly set
    {
        this._store["Prop1"] = value;
    }
}

When readonly is applied to the property syntax, it means that all accessors are readonly.

public readonly int Prop2
{
    get
    {
        return this._store["Prop2"];
    }
    set
    {
        this._store["Prop2"] = value;
    }
}

Readonly can only be applied to accessors which do not mutate the containing type.

public int Prop3
{
    readonly get
    {
        return this._prop3;
    }
    set
    {
        this._prop3 = value;
    }
}

Readonly can be applied to some auto-implemented properties, but it won't have a meaningful effect. The compiler will treat all auto-implemented getters as readonly whether or not the readonly keyword is present.

// Allowed
public readonly int Prop4 { get; }
public int Prop5 { readonly get; }
public int Prop6 { readonly get; set; }

// Not allowed
public readonly int Prop7 { get; set; }
public int Prop8 { get; readonly set; }

Readonly can be applied to manually-implemented events, but not field-like events. Readonly cannot be applied to individual event accessors (add/remove).

// Allowed
public readonly event Action<EventArgs> Event1
{
    add { }
    remove { }
}

// Not allowed
public readonly event Action<EventArgs> Event2;
public event Action<EventArgs> Event3
{
    readonly add { }
    readonly remove { }
}
public static readonly event Event4
{
    add { }
    remove { }
}

Some other syntax examples:

Expression bodied members: public readonly float ExpressionBodiedMember => (x * x) + (y * y);
Generic constraints: public static readonly void GenericMethod<T>(T value) where T : struct { }

The compiler would emit the instance member, as usual, and would additionally emit a compiler recognized attribute indicating that the instance member does not modify state. This effectively causes the hidden this parameter to become in T instead of ref T.

This would allow the user to safely call said instance method without the compiler needing to make a copy.

The restrictions would include:

The readonly modifier cannot be applied to static methods, constructors or destructors.
The readonly modifier cannot be applied to delegates.
The readonly modifier cannot be applied to members of class or interface.

Drawbacks

Same drawbacks as exist with readonly struct methods today. Certain code may still cause hidden copies.

Notes

Using an attribute or another keyword may also be possible.

This proposal is somewhat related to (but is more a subset of) functional purity and/or constant expressions, both of which have had some existing proposals.

LDM history:

tannergooding commented 6 years ago

FYI. @jaredpar

tannergooding commented 6 years ago

This would allow better use of in with things like System.Numerics.Matrix4x4, which is big enough that it is sometimes better to pass around using in (due to size -- 64 bytes), but which can't be marked readonly due to a design-decision to make the fields public and writeable (even if the majority of methods exposed don't modify state).

tannergooding commented 6 years ago

It isn't mentioned above, but it may be worth discussing if a readonly method should only be able to call other readonly methods...

Today, with readonly fields of non-readonly structs, C# will make a copy of the struct. For example, the following prints 0 twice, rather than 0, 1: https://sharplab.io/#v2:C4LglgNgNAJiDUAfAAgZgAQCcCmBDGA9gHYQCe6AzsJgK4DGw6AGgLABQA3u+j+mlnkIlyATXSkA3O269+yACzoAytmAiAFAEoZPLm14HxAOhVr1ARiMAGAGaap+3gF92Ltu35VaDdCPZ6DfhsIAlxGADdcCBpsBwMdPgwFZVUNYNCIqJjtR10Eg2AACzAKI0jo7ABedHKYuOdXaTY5cwA2PgAmPnMAdn8Elvbk5HMADnURqwBtAF10XEwAcwocgwDDHiZ0AA90aqJsAHdmLXqNkaMRgE51baNSMqzse3zeV5470w0X3MML69u90eFR+BjcTiAA=

HaloFour commented 6 years ago

Conceptually I like it, a lot. I do have some issues with it, though.

First, such an attribute already exists, System.Diagnostics.Contracts.PureAttribute, which is supposed to mean the same thing but isn't enforced by the compiler. Adding another attribute to support this feature would be noise.

There's also the chicken&egg problem in that the BCL and ecosystem would have to be updated for this to be particularly useful otherwise these methods are stuck not being able to call out to other methods.

Next there is the issue of what defines "pure". In this case we could keep it to situations where the compiler can safely not make defensive copies, but philosophically it extends beyond that. Perhaps readonly as a keyword would help to divorce this concept from "purity", although they do seem to cover the same bases.

Lastly, readonly is already a valid modifier on the return value of a method. The position is different, but that might still lead to confusion, especially since most of the time modifier keywords can be placed in any order.

tannergooding commented 6 years ago

Adding another attribute to support this feature would be noise.

We would have to add another attribute, however. The existing attribute has no enforcement, as you indicated, and as such would be a breaking change to begin enforcing it now.

There's also the chicken&egg problem in that the BCL and ecosystem would have to be updated for this to be particularly useful otherwise these methods are stuck not being able to call out to other methods.

This is only applicable if we restrict the ability to only call other readonly methods (https://github.com/dotnet/csharplang/issues/1710#issuecomment-404659342)

There could be a difference between a readonly and a pure method. A pure method, by definition, cannot modify any state. A readonly method (in order to satisfy the in constraint and elide the copy) must only guarantee that it does not modify instance state (it could be allowed to do things like call Console.WriteLine, which would allow copy elision, but would not be functionally pure).

It can be summed as All pure methods are readonly, but not all readonly methods are pure.

Depending on what LDM thinls, it could go one way or the other. If we did differentiate, then pure functions would be a natural stepping stone from readonly (and constexpr would again be a natural stepping stone from pure).

Lastly, readonly is already a valid modifier on the return value of a method. The position is different, but that might still lead to confusion, especially since most of the time modifier keywords can be placed in any order.

Right, I called this out in the notes section.

theunrepentantgeek commented 6 years ago

I'm 85% sure this has all been thrashed out before - but I'm unable to find it with a quick search.

One of the real problems is that different developers will have different expectations. Some will expect this to mean that literally no changes are made, none at all, by the method. Others will expect this to mean that no externally visible changes are made.

The former is rigorous and useful from both theoretical and practical perspectives. The later permits result caching (which can be really useful for performance) and other internal changes.

Many of the original decisions in the design of C# were driven by a desire to avoid versioning issues as code evolves - this is, for example, one of the reasons why checked exceptions were not included.

What restrictions on the evolution of my code do I have if I have a property declared like this:

    public string AccessKey
    {
        readonly get { ... }
    }

My own opinion is that I don't think that adding internal caching should have any effect on the external interface; if I need to remove the readonly modifier to add caching, that could have very large cascading effects - and if it's a published API (e.g. via NuGet) then I might not be able to do it at all without breaking people.

That said, I know smart developers who disagree with me fervently on this topic ... 😁

theunrepentantgeek commented 6 years ago

Another scenario to consider ... if you have something with a pure API, and then you modify it to generate audit logs (for legal compliance reasons), should you need to change the declaration of the API?

tannergooding commented 6 years ago

@theunrepentantgeek, right. pure has a number of additional considerations. However, the original post just covers readonly methods, which would not have many of these considerations.

At its root, a readonly method (as proposed) would only guarantee that instance state isn't modified (which would imply calls to System.Console.WriteLine, while not pure, would be allowe).

tannergooding commented 6 years ago

Added a root note, at the top of the post, clarifying that readonly method != pure method

jaredpar commented 6 years ago

@tannergooding

Provide a way to specify that methods do not modify state.

Consider instead

Provide a way to specify individual methods on a struct do not modify state in the same way that readonly struct specifies no instance method modifies state.

jaredpar commented 6 years ago

@tannergooding

Using the readonly keyword is useful, since it is an existing keyword. However, it may get confusing to read if you have something like public readonly ref readonly float GetRefToX() (const T* const anyone 😄)

It's not just that it's confusing, it's also ambiguous with other potential features. The compiler specifically chose to use ref readonly instead of readonly ref for locals / returns because we wanted to allow for the future addition of readonly ref readonly local variables once readonly locals are introduced.

The individual variations are the following for locals in that context:

ref: a reference which can be assigned to and reassigned to a new location
ref readonly: a reference which cannot be assigned to but can be reassigned to a new location
readonly ref: a reference which can be assigned to but cannot be reassigned to a new location
readonly ref readonly: a reference which cannot be assigned to or reassigned to a new location

I wouldn't want to use readonly as proposed here because it would create a bit of confusion between the two cases: specifying a readonly method and readonly locals.

This feature was discussed in LDM when we discussed the readonly struct feature. At the time the syntax we were considering is the following:

struct S1 { 
  int i; 
  public void M() readonly { ... } 
}

That is putting the readonly modifier after the method signature. This is unambiguous and fairly easy to read. This is how we actually implemented permissions in Midori and it works out fairly well. Admittedly it does take some getting used to though for anyone who doesn't have a C++ background.

tannergooding commented 6 years ago

After the method seems reasonable to me. Will update the proposal later tonight.

tannergooding commented 6 years ago

@jaredpar, where would you envision the keyword for properties?

Is it before or after the get keyword (changed it to after in the OP):

public float LengthSquared
{
    get readonly
    {
        return (x * x) +
               (y * y);
    }
}

// vs

public float LengthSquared
{
    readonly get
    {
        return (x * x) +
               (y * y);
    }
}

I would imagine that specifying it individually on the get/set is preferred here, since set (in most normal API designs) wouldn't qualify for the attribute 😄

jaredpar commented 6 years ago

@tannergooding indeed it would be on the get and set as they are separate methods that can have separate mutating semantics.

There is no ambiguity here hence readonly can appear before or after the get / set. I'd lean towards after for consistency.

since set (in most normal API designs) wouldn't qualify for the attribute)

It actually applies in more cases than you would expect. There are a non-trivial number of structs which are just wrappers around reference types. Such structs could themselves be easily marked as readonly, or have a readonly set as they don't modify the contents of the struct.

Starnick commented 6 years ago

I really hope this is considered. I was excited about ref-readonly/in...until I ran into the defensive copy problem. An all-or-nothing approach (declaring the struct readonly) makes using the ref-readonly feature difficult in my context (game development / graphics).

I have a math library that is similar as the examples discussed - low level and mutable math types. Historically, a source of bugs have been when a function takes a math type as a ref, but the parameter is supposed to be readonly (basically have always used the honor system...). Also, while the types are mutable, more often than not the context they are used are in readonly collections (e.g. rendering that acts on the data, the data can only change during a simulation step...so its a "readonly view of the data"); having those collections return a writable reference breaks the design. So I was excited about having more options to express design intent and reduce errors with ref-readonly.

To get around the defensive copies, however, has become something of a pain. I'm considering using extension methods to "fake" instance methods, but then there aren't options for instance properties/indexers.

Admittedly it does take some getting used to though for anyone who doesn't have a C++ background.

I would argue most developers who would be in need of this feature are already pretty familiar with const methods in C++.

jaredpar commented 6 years ago

@Starnick

An all-or-nothing approach (declaring the struct readonly) makes using the ref-readonly feature difficult in my context (game development / graphics).

Thanks for the examples provided here. This matches up with the other asks we've seen for this.

One of the biggest items that held us from doing this at the same time we did readonly struct was a lack of motivating samples. The language feature itself is fairly straight forward but it wasn't clear the extra complexity was justified. Examples like this help us prioritize this properly.

4creators commented 6 years ago

Admittedly it does take some getting used to though for anyone who doesn't have a C++ background

I would argue most developers who would be in need of this feature are already pretty familiar with const methods in C++.

While readonly is used in many places in C# I would prefer to have a bit more clarity with using already existing const keyword after method name declaration i.e.:

struct S1 { 
  int i; 
  public void M() const { ... } 
}

Otherwise proposal seems to be very useful in multiple scenarios and it would be very nice to have it available sooner rather than later.

HaloFour commented 6 years ago

@4creators

IMHO I'd rather save const for possibly writing const-expression methods which are evaluated by the compiler at compile-time.

4creators commented 6 years ago

IMHO I'd rather save const for possibly writing const-expression methods which are evaluated by the compiler at compile-time.

@HaloFour IMHO it could be reconciled with const-expression methods but obviously it is syntactic detail which will be discussed last :)

jaredpar commented 6 years ago

@4creators, @HaloFour

I don't like using const here because the behavior isn't in line with what const means today. Specifically const today refers to a value that can be expressed as a literal, manipulated at compile time and round tripped through metadata by the compiler. These don't describe the behavior of the methods here.

The reason for using readonly here is the implementation lines up with existing uses of readonly in the language today. The other potential modifier to use here is in. After all this feature can best be described as saying:

The this parameter of a struct method annotated with readonly has a type of in T instead of ref T.

There is a good case for in and readonly here. The choice in LDM was for readonly as it's what we use as the type modifier toady hence makes sense at a method level. If we had chosen in struct instead of readonly struct I imagine we would have picked in for this as well.

qrli commented 6 years ago

I'm all in for this feature. Without it, the in feature does not help the Vector/Matrix in game/graphics at all.

And, in C++, if you call a mutable method on a const object, you get an error. In current C#, you get a silent copy. This contradicts with the goal, if I understand correctly, the feature is all for performance.

It actually applies in more cases than you would expect. There are a non-trivial number of structs which are just wrappers around reference types.

In case the struct just wraps a reference, it does not need in at all. The argument on stack is the same size. And the performance could be worse because of the extra mem lookup.

acple commented 6 years ago

What about an expression-bodied getter/method ? public float LengthSquared readonly => ...?

Starnick commented 6 years ago

If we had chosen in struct instead of readonly struct I imagine we would have picked in for this as well.

I'm really glad readonly struct was chosen over in struct, the former reads so much better (at least to a native english speaker).

@qrli

In case the struct just wraps a reference, it does not need in at all. The argument on stack is the same size. And the performance could be worse because of the extra mem lookup.

The simple case where the struct wraps a single reference type, sure. Might be an issue when the struct holds a number of references. But that case is probably exceedingly rare.

@theunrepentantgeek, @jaredpar

My own opinion is that I don't think that adding internal caching should have any effect on the external interface; if I need to remove the readonly modifier to add caching, that could have very large cascading effects - and if it's a published API (e.g. via NuGet) then I might not be able to do it at all without breaking people.

qrli's comment about C++ const correctness made me think about this. Annotating ready-only methods to avoid silent copies is really a struct-only feature, and would basically be useless on a method for a reference type (at least I think so?). So the worry about caching in a reference type would be moot in my opinion, but for structs it certainly has interesting implications.

The case where a struct has to cache some state in a read-only method would probably be rare, but I can see it - maybe a geometry intersection algorithm, or the like where you might need to keep track of some state incrementally as data is processed. And I really like the idea of giving more flexibility to using structs everywhere and in more contexts, e.g. low-allocation apps that manages data using large arrays of structs passed around by ref or ref-readonly. Not common in business apps, but certainly common for games.

C++ of course has a solution to this scenario, the mutable keyword. When a field is marked as mutable, it can be modified in a const method. What are your thoughts in bringing this concept to C#?

jaredpar commented 6 years ago

@Starnick

C++ of course has a solution to this scenario, the mutable keyword. When a field is marked as mutable, it can be modified in a const method. What are your thoughts in bringing this concept to C#?

There is already an API available to do this hence I wouldn't want to add a language concept.

void Example(in T value) { 
  ref T refValue = Unsafe.AsRef(in value); 
  ...
}

The Unsafe Type. There are lots of dangerous / powerful helpers there that let you subvert C# type system rules. Note though that it's the safety equivalent of using unsafe (buyer beware).

jaredpar commented 6 years ago

@acple

What about an expression-bodied getter/method ?

Yes that's the likely syntax.

yaakov-h commented 6 years ago

@jaredpar isn’t that a Core-only API?

tannergooding commented 6 years ago

@yaakov-h, no. It is available on net45 (and a few other platforms as well): https://www.nuget.org/packages/System.Runtime.CompilerServices.Unsafe/

tannergooding commented 6 years ago

@jaredpar, do you want me to submit a PR adding this as a formal proposal.md, or is that something you plan to do as the champion?

jaredpar commented 6 years ago

@tannergooding if you have time that would be great. I'm a couple behind already and this would probably be ~2-3 weeks out for me.

MI3Guy commented 6 years ago

I think it would be worthwhile to add a warning when a non-readonly method is called on a struct that has readonly methods if the compiler would introduce a copy in order to call it.

jaredpar commented 6 years ago

@MI3Guy

I think it would be worthwhile to add a warning when a non-readonly method is called on a struct that has readonly methods if the compiler would introduce a copy in order to call it.

That would introduce breaking changes in existing code. Pretty much a non-starter.

For customers who are interested in this type of warning though I encourage you to take a look at ErrorProne.Net. It has analyzers that already contain this type of warning plus many others you'd likely be interested in.

https://github.com/SergeyTeplyakov/ErrorProne.NET

MI3Guy commented 6 years ago

How so? There are currently no types that have readonly methods. Or are we planning on changing existing methods to be readonly?

Starnick commented 6 years ago

Wasn't there talk some time ago about allowing an opt-in warning for defensive copies?

Opting-in to such a warning because you added a read-only method sounds kind of interesting.

tannergooding commented 6 years ago

Or are we planning on changing existing methods to be readonly?

Technically speaking, all methods on a readonly struct are already readonly methods. We shouldn't have different behavior between those readonly methods and the new explicit readonly methods

jaredpar commented 6 years ago

@MI3Guy

There are currently no types that have readonly methods.

Correct

Or are we planning on changing existing methods to be readonly?

Absolutely. Such a warning would necessarily apply to methods marked directly as readonly and indirectly by adding readonly directly to struct. Otherwise the warning would be inconsistent.

There are already plenty of PRs to make existing types readonly. The most notable of which is likely this mass conversion PR in corefx. Similar PRs are expected for more types if this feature is approved.

The readonly struct feature was designed specifically to ensure such conversions were not breaking changes. The method version will match this approach.

Starnick commented 6 years ago

@jaredpar

There is already an API available to do this hence I wouldn't want to add a language concept.

The Unsafe Type. There are lots of dangerous / powerful helpers there that let you subvert C# type system rules. Note though that it's the safety equivalent of using unsafe (buyer beware).

Ugly, but I see your point. There might be people who don't do this just because it's "unsafe". I'm not a stranger to hacking around the language to get the behavior I want, but it is nice to have a first-class safe way of doing it.

jaredpar commented 6 years ago

@Starnick

Opting-in to such a warning because you added a read-only method sounds kind of interesting.

Problem though is you're not just opting yourself into the warning, you're opting yourself and your customers into the warning.

Starnick commented 6 years ago

@jaredpar

Opting-in to such a warning because you added a read-only method sounds kind of interesting.

Problem though is you're not just opting yourself into the warning, you're opting yourself and your customers into the warning.

Yeah, but if I add new readonly methods or change current methods, aren't I making breaking changes? At my company, we always compile with warnings as errors. If I'm consuming a new version of a library where the type for performance reasons, I should be using readonly XYZ method, I should probably know about it. Although because we compile with warnings as errors, I'm also not necessary looking to die on this hill :)

I just had a thought:

Like in C++, the const-ness is part of the method signature. So you could have two similarly methods that just differ on being const or not const. Does that make sense for C# readonly methods? Or would this not be possible, because the readonly aspect becomes an attribute once compiled?

tannergooding commented 6 years ago

Does that make sense for C# readonly methods? Or would this not be possible, because the readonly aspect becomes an attribute once compiled?

It it theoretically possible, if you use a modreq or modopt. However, I don't believe this is a goal since there are no special semantics to consider here and we want to be able to modify existing methods.

tannergooding commented 6 years ago

Proposal doc is up here: https://github.com/dotnet/csharplang/pull/1730

jaredpar commented 6 years ago

@Starnick

Yeah, but if I add new readonly methods or change current methods, aren't I making breaking changes?

Changing an existing struct or method to be readonly is not a breaking change assuming the code compiles without error. This means the modifier is just enforcing the existing behavior of the type. The only behavior change is the compiler can elide copies at the call site in a number of cases.

This change at the callsite is still observable in a few cases: bad multi-threaded code, unsafe, etc ... But those don't meet our bar for breaking change in the compiler or .NET guidelines.

Although because we compile with warnings as errors, I'm also not necessary looking to die on this hill

The compiler team considers warnings to be equally as breaking as errors. There are just too many customers out there that use warn as error (as your company does) or others that simply require a clean compilation irrespective of that setting. Every time we've attempted to say "it's just a warning" we've gotten strong feedback from a lot of customers that we "broke their build". This can be immensely frustrating at times but it's also how things are.

So you could have two similarly methods that just differ on being const or not const.

True and 99% of the time the two methods have exactly equal content. That's not a strength of C++ though, it's a limitation in their type system (and one I'd never want replicate in C#). The C++ const feature could be much more elegantly implemented by extending the language to have parametric polymorphic. This allows for the permission (const or non-const) to be parameterized based on the inputs. Hence you can have a single definition which uses const when the this item is const and elides const when it's not.

But that's digressing a bit. This is a system we implemented in Midori for our C# permission system and it worked great. But probably don't want to rat hole on that in this issue as it's pretty far off topic.

zpodlovics commented 6 years ago

What would be the concurrency / memory model here? How this supposed to work with managed pointers that points to unmanaged memory [1]? State changes could be also introduced by memory barriers (eg.: reading from a memory address).

[1] https://github.com/dotnet/coreclr/issues/916#issuecomment-98583550

roji commented 6 years ago

Note related discussion (https://github.com/dotnet/csharplang/issues/1157) whose title is about pure but which also discusses readonly.

One additional comment: the above discussion mentions readonly methods of structs (i.e. avoiding the defensive copy problem), but I think there's no reason not to allow this for classes as well. At the very least there's considerable documentation value in knowing that a method doesn't change any state.

CyrusNajmabadi commented 6 years ago

At the very least there's considerable documentation value in knowing that a method doesn't change any state.

"readonly" doesn't imply state is not changed :)

I think that's why it's good to start the feature off small with clear and valuable semantics. In the future it can be expanded out. But it should be done fully thinking things through and making sure the semantics are what are wanted.

JeremyKuhne commented 6 years ago

This came up in #1502 for dealing with fixed arrays as idea 3.

I'm also finding I need this for BufferReader (another large struct). Making a fast reader over ReadOnlySequence<byte> is tricky as we ideally want to cache the ReadOnlySpan<byte> as stashing the ReadOnlyMemory<byte> and repeatedly getting .Span has undesirable overhead.

Doing this necessitates making the mutable BufferReader ref, which has the inadvertent side-effect of preventing us from taking a stackalloc Span without contortions to force a copy of the BufferReader (via using an extension method).

https://github.com/dotnet/corefxlab/blob/master/src/System.Buffers.ReaderWriter/System/Buffers/Reader/BufferReader_binary.cs#L48-L52

In theory this feature would allow us to make Peek() an instance method that can take a stackalloc'ed Span().

tannergooding commented 6 years ago

Updated the OP to match the proposal.md

svick commented 6 years ago

@tannergooding I read the proposal and I'm confused by this part:

Generic constraints: public static void GenericMethod<T>(T value) readonly where T : struct { }

How is this feature related to constraints? And how does it make sense to have a static readonly method? (Especially since that's explicitly forbidden few lines below.)

tannergooding commented 6 years ago

nd how does it make sense to have a static readonly method? (Especially since that's explicitly forbidden few lines below.)

Fixed.

How is this feature related to constraints?

It is showing where the keyword is placed, relative to the constraint. I added a clarifying comment.

gafter commented 6 years ago

This was requested in #1502 (item 3).

jaredpar commented 6 years ago

Update: this was discussed in LDM today. Notes will be out later but in summary the proposal is a go but LDM prefers the readonly syntax appearing in the standard modifier list, not after the closing paren of the signature.

readonly override string ToString()

In a future where C# allows for readonly locals this does mean readonly ref will have different meanings based on the context:

method signature: method that returns a ref where the method has readonly semantics.
method body: local which has a ref type that is not re-assignable

dotnet / csharplang

Champion: Readonly members on structs (16.3, Core 3) #1710

Readonly Instance Methods

Summary

Motivation

Design

Drawbacks

Notes