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

readonly members on interfaces #3055

Open
IS4Code opened this issue Dec 26, 2019 · 45 comments
Open

readonly members on interfaces #3055

IS4Code opened this issue Dec 26, 2019 · 45 comments

Comments

@IS4Code
Copy link

IS4Code commented Dec 26, 2019

I think readonly members as introduced in C# 8.0 (#1710) should be usable on members of interfaces as well. The main reason is in this example:

public interface IVector<TVector> where TVector : struct, IVector<TVector>
{
    readonly TVector Add(in TVector other);
}

TVector Add(in TVector a, in TVector b) where TVector : struct, IVector<TVector>
{
    return a.Add(b); // shouldn't cause copying
}

If readonly members on interfaces aren't available, there is no way to make a generic method for vector adding (or other operations) without copying. This is extremely useful once you start working with larger structures that are composed of multiple smaller types (like vectors and matrices). For example, if Vector contains 4 float values, Vector<Vector> then contains 8 values, Vector<Vector<Vector>> 16 values; each containing two fields of the inner type:

public readonly struct Vector<TInner> where TInner : struct, IVector<TInner>
{
    readonly TInner first;
    readonly TInner second;

    public readonly Vector<TInner> Add(in Vector<TInner> other)
    {
        return new Vector<TInner>(first.Add(other.first), second.Add(other.second)));
    }
}

Without readonly on IVector<TVector>.Add, this code must copy first and second before calling Add on the inner type, and there is no other way without abandoing readonly and in at all to prevent this, other than providing a separate object for operations with TVector Add(in TVector, in TVector) method, at the cost of a virtual call. Marking the interface method readonly means that the compiler knows the in parameter will not be modified (at least by an implementation in C#).

readonly should be allowed on interface members to indicate that the implementing method must honor readonly as well. Then, if the implementing type is a struct, it must implement any readonly method with another readonly method. The other way around should be possible though, i.e. implementing a non-readonly method by a readonly one.

Relation to classes and #3885

Calling a readonly method on structs already has a well-defined meaning: this won't be modified from inside of the method. This is the meaning that is extended here to interfaces, but classes cannot change the value of this, so in essence all methods are implicitly externally readonly. A class can therefore freely implement an interface with readonly methods without caring about readonly anywhere in the implementation, because this will never be modified. A readonly variable of a reference type never requires creating a defensive copy when calling any of its methods, and is a non-issue if used in the example above.

This proposal is, for the most part, unrelated to #3885, but what needs to be resolved is whether readonly on an interface should denote external readonlyness (this is not observably modified) or internal readonlyness (can't write to fields and call non-readonly methods). Value types share both meanings, because there is no indirection that could separate them. It seems that classes could only have internal readonlyness, unless a C++-style const reference to an object is considered, but then it would warrant a new keyword anyway.

For that reason, even if readonly class was added, the relation to interfaces would still be the same: a class implementing an interface with a readonly method doesn't have to care about upholding readonly on that particular method, because its can never be differentiated from the point of the interface.

@DaZombieKiller
Copy link
Contributor

I believe readonly members would need to be allowed on class types before this could happen.

@IS4Code
Copy link
Author

IS4Code commented Dec 27, 2019

@DaZombieKiller That might not be necessary. There is no real benefit to a readonly class or a class method, as it doesn't have to be pure anyway, and there is no such thing as a readonly object or reference to it. So readonly can be specifiable on classes with the same rules as for structs, or not, and then you would be able to implement a readonly interface method with a non-readonly one in a class.

I am for the second option. If people start placing readonly on classes or their methods because they think the code is safer or faster that way, that only shows a misunderstanding of the keyword. Make it explicit that having a readonly interface member is there only for implementing structs, and classes could ignore it.

@jaredpar
Copy link
Member

jaredpar commented Jan 1, 2020

Sounds like the request is roughly the following:

  1. Allow the readonly modifier to appear on interface member declarations.
  2. When a struct implements an interface where members have the readonly modifier, the matching methods on the struct must also have readonly. Or the declaration must be a readonly struct which implies readonly on every member
  3. When a readonly member is invoked on a value which is a type parameter than is also constrained to be a struct then elide copies where you would elide them for readonly members on a struct.

That's a reasonable extension of the readonly feature. It's also a request that I've seen before (with slightly different semantics). I'm unsure if there are enough places that this would be useful to justify doing it.

Consider that when we did the readonly struct feature originally we talked about allowing individual members also be marked as readonly. That was not done though, in part, because we couldn't find enough types / data to justify adding that extra level of granularity. After we released readonly struct though developers provided enough examples that we felt it was worth the extra level of granularity and concept count to the language.

I think we'd need to see similar examples or patterns to get this through LDM.

I believe readonly members would need to be allowed on class types before this could happen.

Not necessarily. The readonly member feature is specific to struct types. The rules could just be written to ignore readonly when implementing the interface on a non-struct type.

@IS4Code
Copy link
Author

IS4Code commented Jan 2, 2020

@jaredpar Yes, that is pretty much it. Not sure about others, but I have stumbled upon this when doing what was pretty much my initial example. It would also give this issue a reason to be resolved.

@DavidArno
Copy link

I believe readonly members would need to be allowed on class types before this could happen.

Not necessarily. The readonly member feature is specific to struct types. The rules could just be written to ignore readonly when implementing the interface on a non-struct type.

This would create issues were readonly/pure classes ever to be implemented in the future. We'd have the situation where an interface could declare a readonly member, as could a class. But there'd either be no requirement for the class to respect the readonly requirement of the interface, or you'd have to make a massive breaking change to enforce that contract causing existing code to fail.

@HaloFour
Copy link
Contributor

HaloFour commented Jan 6, 2020

@DavidArno

IMO readonly already means something other than "pure" in the C# language so I doubt any such feature would be based on that keyword.

@IS4Code
Copy link
Author

IS4Code commented Jan 6, 2020

@DavidArno
The whole concept of readonly is to prevent a location from being modified. readonly structs and methods are here to allow operating on read-only data without having to create a defensive copy each time, as the compiler must ensure that even though it produces unverifiable code (loading the address of a read-only field), the code is still valid. There is no such thing for classes – the object's mutability is governed purely by its implementation, and having a readonly (private) field in a class or readonly method is as useful as having a readonly local variable or a parameter – to make sure that you don't modify it.

There is nothing like a readonly object neither in the language nor in the runtime, there is no mechanism for producing the defensive copy of an object, and most importantly, readonly can be completely bypassed if you move your class's storage (to another object or a dictionary, for example).

Concepts like "read-only" and "pure" are independent on each other, and aren't even in any subset relation. Something to indicate mutability (the observable state of the object is not modified) or purity (the observable state of anything is not modified) could be useful in the future, but it's not related to readonly.

@DavidArno
Copy link

There is nothing like a readonly object neither in the language nor in the runtime...

And there was nothing like a readonly struct before the concept was introduced.

Immutable types (ie ones that go beyond having read-only class fields where the ref can't change to guaranteeing the content of the things pointed to by those refs are read-only too) would be a useful addition to the language. Likewise having read-only methods that do not change the (direct) value of the object's state would also be useful (you are right that these aren't quite pure methods as they cannot guarantee no side effects without truly immutable types).

This might never happen of course as there are so many other features vying for the language team's attention. But it would be disappointing to see this effectively ruled out completely simply because a change was made to support structs that then rendered it so much harder to implement read-only/immutable classes in future.

@jaredpar
Copy link
Member

jaredpar commented Jan 6, 2020

I doubt that we would ever add readonly to class types in the future because it really has no inherent value. The value proposition for struct is quantifiable: type states implementation detail as contract so consumers can avoid introducing unnecessary defensive copies. The value proposition for class would be: type states implementation detail. Not really a big seller.

The reason is that readonly is a shallow guarantee. That's true even for struct as a struct nested in another struct is really just putting a name on a group of fields. The readonly of the containing struct ends up dominating the design of the nested struct. That has the bonus though that we can avoid defensive copies with a single readonly on the containing type.

The shallow guarantee for a class though doesn't really provide any value. What does it matter if a method implementation in a class modifies it's immediate field vs. a nested field? To the consumer it's still a modification and there is no specific optimizations or really any semantic changes you could make based on that information.

If we added a modifier to class it is much more likely to be immutable. That is essentially the readonly guarantee but deep., Meaning an immutable type can only contain other types which are immutable. That is a meaningful guarantee that consumers can optimize around.

@IS4Code
Copy link
Author

IS4Code commented Jan 6, 2020

Just an example of how readonly on classes can be bypassed.

readonly class Class
{
    readonly StorageClass storage = new StorageClass();

    public int Property {
        get {
            return storage.Property;
        }
        set {
            storage.Property = value;
        }
    }

    class StorageClass
    {
        public int Property;
    }
}

@DavidArno
Copy link

@IllidanS4,

Just an example of how readonly on structs can be bypassed:

    readonly struct Struct
    {
        readonly StorageClass storage;

        public int Property {
            get {
                return storage.Property;
            }
            set {
                storage.Property = value;
            }
        }

        class StorageClass
        {
            public int Property;
        }
    }

As I previously mentioned, immutable types, that do not permit such bypassing, would be a useful addition to the language. That could be achieved by only allowing readonly types to be used within a readonly class. But this would create inconsistencies with readonly structs and the latter could still be used to bypass the restriction via the above example.

As @jaredpar mentioned, a new keyword - eg immutable will likely be needed for types and fields to indicate that not only is the reference immutable, but the entire type - all the way down - is immutable too. Just as it sometimes makes sense now to mark a non-read-only struct's methods as readonly, so that existing keyword could be used to denote that a class method doesn't access mutable state either.

So therefore, introducing readonly methods to interfaces before the whole subject of immutable types has been thought through risks making immutable types more difficult to implement.

@IS4Code
Copy link
Author

IS4Code commented Jan 7, 2020

Thinking about it, externally readonly method means "this method will not modify the variable where it is called". In this sense, all class methods are readonly as the this reference itself will never be modified.

This is especially apparent with generics:

public static string TestFormat<T>(in T arg) where T : IFormattable
{
    return arg.ToString(null, null);
}

The compiler has to ensure that the value of arg shall never be modified, so under the hood it creates a defensive copy. However, this doesn't have to happen in two cases: if T is constrained to a class, so arg is an object reference and thus can be modified only by direct assignment to arg; or if IFormattable.ToString is declared readonly. If both are true, readonly is superfluous, so technically all reference types are already readonly (in the terms of references to their instances). This also means that if where T : readonly is introduced in the future, it should match readonly structs but also all classes.

@jaredpar
Copy link
Member

jaredpar commented Jan 7, 2020

Just an example of how readonly on structs can be bypassed:

This doesn't bypass readonly on a struct though. The readonly modifier, both on the struct declaration and in storage locations such as fields, describes the state of the memory / location of the fields. Specifically that the location cannot be altered through that reference.

The readonly modifier was never intended to protect against modifications to reference types that were reachable from a struct. That is true all the way back to C# 1.0.

Saying that this approach bypasses readonly requires an incorrect definition of the feature.

@Thaina
Copy link

Thaina commented Apr 4, 2020

While this might be logically plausible. @DavidArno concern is valid. I think the idea that keyword is already there but ignoring class is a bit concerned me

So if readonly interface member need to be implement anyway. I think this might be better if we wait for shape and allow us to define readonly shape as a constraint instead? That way we don't need to change the old interface to support this feature

But then again, Maybe we could just use another keyword (maybe new one or repurpose the old one) for pure class and member, and left readonly as it is, or making readonly a super/sub keyword of that another keyword (readonly will ignore class but that another keyword work on both class and struct)

@IS4Code
Copy link
Author

IS4Code commented Apr 6, 2020

I think we should leave the concept of purity or the actual soundness of readonly to other proposals. The aim of this is simple: to be able to accept readonly values that implement a specific contract simply and efficiently. So something like this would be finally possible, as a result:

interface IBetterFormattable
{
    readonly string ToString(string format);
}

static int GetSpecificStringValue<T>(this in T arg) where T : struct, IBetterFormattable // "struct" also shoudn't be technically needed
{
    return arg.ToString("some format"); // no defensive copying for *any* input
}

@agocke
Copy link
Member

agocke commented Jul 4, 2022

I just hit this as well. Seems like a simple, worthwhile feature. It basically means I can provide my users the following API:

public class C
{
  public void M<T>(in T t) where T : IMyInterface
  {
      t.Run();
   }
}
public interface IMyInterface
{
  readonly void Run();
}

and then the callers of C.M won't copy if the input is a struct. The whole point of providing in in the first place is to avoid copying, but as far as I can tell there's no way to avoid it right now without readonly interface methods.

@bernd5
Copy link
Contributor

bernd5 commented Jul 6, 2022

As a hack you can use "System.Runtime.CompilerServices.Unsafe.AsRef" to avoid copies - but it is unsafe - you can call non readonly methods...

using System;

var f = new F()
{
    Value = 12
};

C.M(in f);
Console.WriteLine(f.Value);

public class C
{
  public static void M<T>(in T t) where T : IMyInterface
  {
      ref T tr = ref System.Runtime.CompilerServices.Unsafe.AsRef(in t);
      tr.Run();
      tr.Run2(); //this violates "in" - t is modified
   }
}

public interface IMyInterface
{
   void Run();
   void Run2();
   public int Value { get; set; }
}

public struct F : IMyInterface
{    
    public readonly void Run(){
    }
    public void Run2(){
        Value++;
    }
    public int Value { get; set; }
}

readonly interface members would be great for structs - but what would that mean for implementing classes (there we have no readonly methods, too)?

@IS4Code
Copy link
Author

IS4Code commented Jul 6, 2022

readonly interface members would be great for structs - but what would that mean for implementing classes (there we have no readonly methods, too)?

I have said that already in the first post: a readonly method can be easily implemented in a normal class without any specific keyword, because (externally) all methods on a class are implicitly readonly.

Perhaps better to illustrate with an example:

interface I
{
    readonly void M();
}

struct S : I
{
    public C c;

    public readonly void M()
    {
        c.M();
    }
}

class C
{
  void M();
}

S.M can be readonly, because it does not change the reference when C.M is called. But all that S contains is a reference to C, so it should be possible to simply use C in the first place without an intermediate struct. Calling C.M on any reference never changes the immediate value (the reference itself) which is what readonly is about.

@jaredpar
Copy link
Member

jaredpar commented Jul 6, 2022

@agocke

I just hit this as well. Seems like a simple, worthwhile feature.

It's simple if you just consider the current landscape of readonly. That is that it only applies to struct. There are several proposals out there to have readonly apply to class in a similar fashion. Taking this proposal first essentially decides, in the negative way, about ever doing readonly on class. At the point readonly exists in interface it becomes a massive compat issue to start applying readonly to class as it would break exsisting interface implementations.

@IS4Code
Copy link
Author

IS4Code commented Jul 6, 2022

@jaredpar Frankly I still don't see any compatibility issue here.

The meaning of readonly is fixed for structs and their methods ‒ externally it signalizes that the method won't change the value of this (making it safe to use for readonly references) and internally it prevents you from modifying the fields of the struct.

On interfaces, readonly cannot prevent you from modifying any of the fields (because there are none). What absolutely remains to be decided though is how default interface methods should interact with it. I assume this is boxed there, so readonly methods should be allowed to call non-readonly methods freely (unless there is a difference between this.M(); and var c = this; c.M(); ‒ what type would c be to prevent you from calling c.M?).

I don't really see why or how a class could uphold readonly on an interface in any way that is sensible to someone who uses it through that interface. What would the "value" of an interface's instance be anyway? All values of its properties? Just have a property that always evaluates to Guid.NewGuid() and you'll have a "mutable" state without breaking any possible readonly constraint anyway, unless C# turns into a pure functional language.


The proposals for readonly classes have been left undecided for years, so why not decide at least something which actually makes two parts of the language usable together? I say classes should completely ignore any readonly attributes placed on their interfaces. Show me why they shouldn't.

@jaredpar
Copy link
Member

jaredpar commented Jul 6, 2022

@IllidanS4

Frankly I still don't see any compatibility issue here.

The effect of readonly is that it forces implementing members to have readonly if applicable. Today for struct that has meaning and prescribes a specific behavior. This proposal ignores class because readonly is not a valid modifier.

It's possible in the future that readonly becomes a valid class declaration and member modifier. There have been many such proposals for that over the years in a couple different forms. If that proposal was ever implemented then suddenly readonly has meaning for class and it becomes a compat issue for any class that implemented a interface with one or more readonly members. At the very least it's a source breaking change when that happens.

This is something that has to be thought through and considered. This is why I reject the idea that it's a simple feature to implement.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

@jaredpar What exactly does it mean that "this proposal ignores class"? Both in the initial post and in my two previous posts I have given an example how this could be easily resolved without interfering with any class readonly proposal. It's simple ‒ just ignore readonly on interfaces when implemented by a class, now and in the future, because that makes the most sense. I've stated this over and over yet somehow I forget that classes exist.

It's an irony that two years ago the "blocker" for this was, for some reason, that readonly on classes was not considered:

I doubt that we would ever add readonly to class types in the future because it really has no inherent value.

Yet now the issue is that readonly on classes is considered and has priority over this proposal. All while I keep repeating that they are independent on each other.

@HaloFour
Copy link
Contributor

HaloFour commented Jul 7, 2022

@IllidanS4

Yet now the issue is that readonly on classes is considered and has priority over this proposal. All while I keep repeating that they are independent on each other.

It's not about priority, it's about the fact that they are dependent on one another. Implementing readonly on interface members today certainly impacts the ability for the language team to define what it would mean for readonly to be applied to class members, or whether the former would require the latter. The team can't "just ignore it" as it effectively forces their hand on other decisions, and clearly they're not comfortable simply dropping one for the other.

@CyrusNajmabadi
Copy link
Member

CyrusNajmabadi commented Jul 7, 2022

It's simple ‒ just ignore readonly on interfaces when implemented by a class, now and in the future

This is not palatable to me. Esp. if we ever actually have readonly classes. Takign this approach seriously limits future design space. And we do not want to limit future design space unless we're completely sure as a team that that is very much what we want.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

So why or how should a class take readonly on an interface into account? Interfaces don't have any fields or inner workings that even could be protected with readonly.

@CyrusNajmabadi
Copy link
Member

So why or how should a class take readonly on an interface into account?

We'd have to design that out.

Interfaces don't have any fields or inner workings that even could be protected with readonly.

Sure. But classes would. So potentially to respect that, a class might have to not mutate itself in any of tehse members.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

So potentially to respect that, a class might have to not mutate itself in any of tehse members.

How is that useful to users of the interface? I cannot distinguish whether a class mutated itself or not from the perspective of an interface. And in any case, I can perform a number of tricks ranging from another inner instance to ConditionalWeakTable that bypasses this. readonly on an interface is useless for a class.

@agocke
Copy link
Member

agocke commented Jul 7, 2022

This is something that has to be thought through and considered. This is why I reject the idea that it's a simple feature to implement.

Agreed, I forgot about language evolution questions.

@CyrusNajmabadi
Copy link
Member

How is that useful to users of the interface?

There are many times i would like to enforce that a type isn't mutating out from underneath me, and having such a constraint would be valuable, even for classes.

readonly on an interface is useless for a class.

Under your set of value propositions that may be true. But we may evaluate under a different set. And those value propositions may change over time.

I can perform a number of tricks ranging from another inner instance

I don't know what this means. I could envision a world, for example, where being readonly might put such a constraint on inner values as well, for deep readonlyness to a type and enforcing restrictions to make mutation potentially impossible (deeply).

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

There are many times i would like to enforce that a type isn't mutating out from underneath me, and having such a constraint would be valuable, even for classes.

I agree, but this is not what readonly is about. We are arguing readonly here and that keyword is for shallow immutability with respect to the fields, not for any other meaning. We've already established that, were this the case, there would be a keyword like immutable with this meaning, as using readonly for that would simply be confusing.

But we may evaluate under a different set. And those value propositions may change over time.

This set is already affected by the existing meaning of readonly for fields and structs/struct methods. Just with the key difference that those aren't hypothetical features; they actually exist.

I don't know what this means. I could envision a world, for example, where being readonly might put such a constraint on inner values as well

We don't have that world and we can't have that world ‒ readonly already means a specific thing and breaking that would be much more horrendous than the slight inconvenience for class readonly proposals this one would pose.

We can't have this discussion if we can't agree what readonly actually means. Could we please stick to what the current version of C# uses it for?

@CyrusNajmabadi
Copy link
Member

I agree, but this is not what readonly is about. We are arguing readonly here and that keyword is for shallow immutability with respect to the fields, not for any other meaning.

That's the state of hte world now. You're asking me to make a stance that that will be the case forever.

  1. that is not something i am comfortable with.
  2. this is exactly a case of limiting future design space. It would force our hands so to speak, even if we came to regret that later.

@CyrusNajmabadi
Copy link
Member

We've already established that, were this the case, there would be a keyword like immutable with this meaning

No. This hasn't been established. It's been argued for. That argument may end up winning out. It's certainly reasonable. But it's not been established fully as a design point that we will base all future designs off of. I, for one, would want to deeply (no pun intended) go into that sort of decision before stating firmly that it's certainly what we want.

@CyrusNajmabadi
Copy link
Member

We don't have that world

We don't have that world... because we're talking about the present. I don't know where our future designs will take us.

and we can't have that world

I don't see why not. The future of hte language is ours to decide where we want to go with. The feature literally doesn't exist, and we can decide what we would want it to mean at the point it came into being.

We can't have this discussion if we can't agree what readonly actually means. Could we please stick to what the current version of C# uses it for?

There is no current meaning for 'readonly' on interfaces, or how it would affect classes. That's the entire point of "a design space". What readonly means in the future is an open topic that may develop and evolve with teh language. The team may decide it's acceptable to restrict taht design space. But it's not a given, and our general approach is to not do that until we've discussed it fully and believe that to be the right decision. Until that happens, the preference is to keep things open so that we don't end up overly restricting things and ending up in a situation we regret.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

That's the state of hte world now. You're asking me to make a stance that that will be the case forever.

You are technically right ‒ something that is not defined today could easily be defined to be absolutely anything tomorrow, but practically, any definition lives surrounded by the already existing definitions, in this case how readonly is already defined in places where it is allowed now. You could define a readonly method on classes to mean "doesn't affect the externally observable state", but that is not what a readonly method on structs means now. In fact, this actually prevents this proposal from being realized because then readonly methods on interfaces would also mean a completely different thing, and it would be also incompatible with structs implementing them. You have the same kind of incompatibility issue if you go either way. With the only difference that readonly struct methods already exist and thus should (in my humble but strong opinion) have priority in a "deadlock" like this.

I don't know what this means. I could envision a world, for example, where being readonly might put such a constraint on inner values as well, for deep readonlyness to a type and enforcing restrictions to make mutation potentially impossible (deeply).

Sure, I can envision that world as well, but readonlyness where defined now means something else. Going for this world would introduce either breaking changes or inconsistency. Neither seems desirable to me.

This hasn't been established. It's been argued for. [...] I, for one, would want to deeply (no pun intended) go into that sort of decision before stating firmly that it's certainly what we want.

You are correct, and I respect this methodology. To me this discussion feels like a part of this decision process, so I hope this is worth it.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

This effectively seems to boil down to one of these situations, should readonly on interface methods mean something in the future:

  1. readonly on class methods means true immutability; thus readonly on interface methods enforces true immutability for classes that implement them. Structs would have to abide by this as well, except that readonly on struct methods already means something else. Should the meaning change when the struct implements an interface with a readonly method? Perhaps, but this seems confusing to me. Additionally this proposal would be impossible (or at least very difficult) to implement.
  2. readonly on class methods means essentially the same as on struct methods. This is completely compatible with this proposal, and what's left to determine is whether class methods should uphold readonly on the interface methods. I don't see a particular reason why this would have to be the case, but either way could work (it can be bypassed anyway).
  3. readonly on class methods is still prohibited, and only this proposal is implemented. This is fine, but you can still proceed to case 2 whenever needed.
  4. readonly on class methods means something else completely. In that case, it cannot interact with readonly methods on interfaces with this proposal, so it is in a sense opposite to case 1. readonly on class methods would mean something different to both struct and interface methods.

Feel free to suggest other situations, if there are more. Case 2 and 3 seem most likely to me; case 4 is hard to imagine (but feel free to prove me otherwise) and case 1 seems too inconsistent to me to be followed with.

If you want to have some sort of process to narrow down the possible cases, I urge you to have it, to start somewhere.

@tannergooding
Copy link
Member

tannergooding commented Jul 7, 2022

There is a legitimate source (and potentially binary) breaking consideration if a decision on how readonly class works isn't made first.

Consider:

interface IFoo
{
    readonly int GetLength();
}

A struct implements this and everything is fine as the implementing method must be readonly.

A class implements this but readonly class doesn't exist. Because it doesn't exist there is no enforcement on behavior and the user mutates, this is "effectively" violating the readonly contract.

Adding readonly class in the future now becomes at least source breaking since the interface implementation wouldn't bind to the mutating method.

Now consider that value types are sealed and the language has used modreq in the past to ensure deriving types correctly understand certain contracts (such as is done for in, ref readonly, and other features when used in a virtual or abstract member). If this was done for a readonly virtual int GetLength(); then it becomes binary breaking as well since the non readonly signature wouldn't have the relevant modreq.

This means that readonly class must be considered, decided upon, and likely exposed (barring a decision to never support it) before this feature is feasible.

It is, IMO, highly probably that readonly class would mean functionally the same thing as a pointer, ref, or array to a readonly struct since they are conceptually similar (1 or more indirections to the actual value). However, there is no guarantee that the language team says "this is the way" and there may be other considerations at play.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

@tannergooding

A class implements this but readonly class doesn't exist. Because it doesn't exist there is no enforcement on behavior and the user mutates, this is "effectively" violating the readonly contract.

This all depends on how you choose to interpret readonly as an external observer of an instance of a type. Perhaps it does need a formal decision to confirm that this is the right way to interpret it, but to me, readonly int GetLength(); (wherever it would occur) implies this contract:

Foo f = ...; // Foo implements IFoo
Foo g = f;
f.GetLength();
// f and g are still identical

This kind of contract really only makes sense for value types, because you do have access to their fields, and in situations like in references, it is your memory whose readonlyness must not be violated. To me, "identical" in the example above means that the memory allocated to the variables f and g stays the same, byte by byte (I acknowledge this is not a proper definition due to the language's abstraction, but I hope you understand what I mean).

Now why do I choose this kind of interpretation? The answer is simple: consistency. A declaration like readonly Foo f has precisely the same meaning as above: the memory corresponding to the variable f is never changed. It is trivial to see that any class method upholds this contract, regardless of whether its fields are modified or not, because they are an implementation detail, fully encapsulated by the class (well unless you make them public of course, but why would their readonlyness need to be a part of a contract when the fields themselves cannot ever be part of a contract?).

Remember that the purpose of using readonly here is to give it the same meaning, externally, as structs have, for the purpose of using in parameters without copying. This is a complete non-issue for classes, and this is why classes don't even have to care about something like that on an interface. An interface doesn't have any fields that need to be protected with a readonly in the first place! Yes, you can police yourself with readonly class methods, and readonly virtual methods do seem to have some merit, but there is no internal state that an interface could ever need to protect with readonly.

And if there is a need to add something similar, but not quite like, to readonly, then all this needs to be solved is to use a different keyword that does not connotate what readonly does.

Now consider that value types are sealed and the language has used modreq in the past to ensure deriving types correctly understand certain contracts (such as is done for in, ref readonly, and other features when used in a virtual or abstract member). If this was done for a readonly virtual int GetLength(); then it becomes binary breaking as well since the non readonly signature wouldn't have the relevant modreq.

That is a valid concern, but it's more of a technical issue. Sure, adding readonly to an interface method would change the signature. The difference is that this is extremely important for structs but completely irrelevant (and a minor nuisance) to classes, but my point was that you'd use readonly if you only cared about structs, so in practice it wouldn't be such an issue. And of course retro-adding readonly to all BCL interfaces would be a terrible idea.

This means that readonly class must be considered, decided upon, and likely exposed (barring a decision to never support it) before this feature is feasible.

Not necessarily to never support it; see my previous post. This proposal does restrict how readonly class could be realized, but the most consistent definition is still permitted to allow whenever it is deemed so. Essentially readonly on interfaces would be unrelated to readonly on classes, and that is the only thing necessary to consider.

It is, IMO, highly probably that readonly class would mean functionally the same thing as a pointer, ref, or array to a readonly struct since they are conceptually similar (1 or more indirections to the actual value). However, there is no guarantee that the language team says "this is the way" and there may be other considerations at play.

Perhaps, but even this interpretation is completely irrelevant from the perspective of an interface, because making all of your class's fields readonly hardly makes the observable behaviour different.

@tannergooding
Copy link
Member

tannergooding commented Jul 7, 2022

This all depends on how you choose to interpret readonly as an external observer of an instance of a type

There is no "choosing" to interpret here as the spec and documentation has very explicitly defined semantics. Irrespective of what someone else observes, the spec wins out 9/10 (the exception generally being esoteric edge case bugs that have existed for 10+ years that can't be fixed due to back-compat). The language designers and compiler devs who work on, define, and implement the language features are effectively the "de facto" source of truth on ambiguities and interpretation. They are also the de-facto source for future direction, considerations, and impact that various things have.

In the case of readonly struct and readonly instance members, the defined semantic is that all fields are readonly (or treated as readonly for the individual method). The observed semantics that fall out from that don't really matter and the semantics fairly clearly map to cases of indirections: MyStruct[], MyStruct*, ref MyStruct, in MyStruct, ref readonly MyStruct, etc. Given that class MyClass is also just a "single indirection" to the backing data and readonly struct impacts the "backing data" (directly contained fields), it is not a huge jump in logic to understand how it could correspondingly impact class nor some of the "edges" that might need to be considered around parameters. The biggest "hole" that I can think of is around hidden copies where making hidden copy is "trivial" for structs and records, but where it is "non trivial" for arbitrary reference types.

This proposal does restrict how readonly class could be realized, but the most consistent definition is still permitted to allow whenever it is deemed so

It's been stated multiple times by the language designers and experts that this proposal being implemented without first considering what readonly means to a class would in fact restrict how readonly class could be exposed and with reasons why. It could be that they decide readonly class doesn't have any inherent value and therefore shouldn't be exposed/supported, but that then comes with the corresponding decision that readonly instance members in interfaces would have no semantic for classes and that any change in stance on readonly class in the future would then be impacted.

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

There is no "choosing" to interpret here as the spec and documentation has very explicitly defined semantics.

Sure, but it has been pointed out before that something that is not yet defined is free to be defined in any conceivable way. I am just stating that if you are in that position to define a new occurrence of a keyword, it makes sense to extend the definition from other contexts. There are multiple ways to do that, so there is a freedom of choice there. To me readonly in all places is shallow (like readonly ValueTuple<StringBuilder> still allows you to modify the inner object), so it also makes sense to me to extend it to a new place in a similar manner, but the freedom of choice is there.

it is not a huge jump in logic to understand how it could correspondingly impact class nor some of the "edges" that might need to be considered around parameters. The biggest "hole" that I can think of is around hidden copies where making hidden copy is "trivial" for structs and records, but where it is "non trivial" for arbitrary reference types.

It is complicated, or at least there are multiple ways to do it. For one using a readonly class doesn't need, in my opinion, any different external handling, unless you'd also want to bring in the concept of a "readonly object reference", like const T* in C++ (compared to T* which is like an object reference). But readonly is not the keyword that could be used there, as readonly StringBuilder already has a meaning.

without first considering what readonly means to a class would in fact restrict how readonly class could be exposed and with reasons why

I didn't say otherwise. Any feature added to the language restricts what additional features could be added, or how. Say you wanted to make readonly StringBuilder actually mean immutable? Too late for that. But surely when readonly was being added to the language, it was discussed whether it should propagate to classes or not, however such a thing was actually decided without accepting/rejecting readonly classes.
It doesn't matter whether readonly class is decided to be added or not; either is possible with or without this proposal.

It could be that they decide readonly class doesn't have any inherent value and therefore shouldn't be exposed/supported, but that then comes with the corresponding decision that readonly instance members in interfaces would have no semantic for classes and that any change in stance on readonly class in the future would then be impacted.

The next step then is to assess how big the impact would be. I don't argue that there is none, but I have already argued that readonly class is absolutely implementable even with this proposal accepted first. The problem is that in 2 years there hasn't even been a concrete proposal how readonly class should precisely work, and so far all reasons for it were "convenience" for "expressing intent". Here usability is at stake, but it seems like the barrier is that something nobody wants to discuss should be discussed first.

@tannergooding
Copy link
Member

The problem is that in 2 years there hasn't even been a concrete proposal how readonly class should precisely work

There are 428 active issues in the repo representing "championed" or "already implemented" language features. There are 25 entries per page * 116 pages and so ~2900 active proposals, questions, feature requests, etc.

The language has many competing issues, priorities, and desires and tends to group things together into themes as well as making overall iterative improvements each release. Some issues can take a few years to get properly prioritized, designed, and implemented. However, even with moving at this pace, C# is still getting massive amounts of innovation each release and improving in leaps and bounds. It is also not moving slower than most other languages, in many cases its moving a bit faster now (especially compared to how it used to progress).

These things take time and as much as you really want readonly members in interfaces, the other 3 million+ C# devs all want their own other things as well and so there is a balance to it all. The first step for this one is really getting it championed (as it doesn't appear to be today). Because of that, it should likely be a discussion and not a regular issue (as per the normal repo rules/guidance).

@IS4Code
Copy link
Author

IS4Code commented Jul 7, 2022

It was not my intention to complain that C# is evolving too slowly, in fact I am grateful for the additions that are being added in, and the pace. The complaint is that the proposal for readonly class members isn't even fully fleshed out yet, so there isn't really much option for me other than to guess what the relation to interfaces would be there, and it's clear some people have a different idea. All it takes is someone equally dedicated to finish that proposal, and we would have something to discuss and consider.

Because of that, it should likely be a discussion and not a regular issue (as per the normal repo rules/guidance).

GitHub discussions weren't a thing 2 years ago 😉. I am not opposing it, but this seems concrete enough.

@tannergooding
Copy link
Member

tannergooding commented Jul 7, 2022

All it takes is someone equally dedicated to finish that proposal, and we would have something to discuss and consider.

and a respective member of LDM championing the issue and driving it to completion. Finding a champion is most often the first step

GitHub discussions weren't a thing 2 years ago 😉. I am not opposing it, but this seems concrete enough.

Yes, but triagers have moved most non-championed issues to be discussions. This one was likely either missed or its potentially incorrectly labeled. @333fred might be able to confirm and transfer if this should in fact be a discussion since it doesn't have a listed champion.

@333fred
Copy link
Member

333fred commented Jul 7, 2022

Unless @jaredpar or @CyrusNajmabadi would want to champion this, it should probably be moved to a discussion.

@jnm2
Copy link
Contributor

jnm2 commented Jul 8, 2022

It would be surprising if readonly had a different effect on a reference-typed variable than it did on a struct-typed variable where the struct contains a single reference-typed field, with respect to how readonly works with reference-typed memory locations.

@IS4Code
Copy link
Author

IS4Code commented Jul 8, 2022

It would be surprising if readonly had a different effect on a reference-typed variable than it did on a struct-typed variable where the struct contains a single reference-typed field, with respect to how readonly works with reference-typed memory locations.

Indeed, that is why I think that readonly members on interfaces do not have any relation to potential class readonly members.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests