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 generic constraint #3691

Closed
juliusfriedman opened this issue Jul 16, 2020 · 48 comments
Closed

readonly generic constraint #3691

juliusfriedman opened this issue Jul 16, 2020 · 48 comments

Comments

@juliusfriedman
Copy link

juliusfriedman commented Jul 16, 2020

Consider

public static T M<T>(T t){ return default; }

Something which is not allowed

public static T M2<T>(T t) where T : readonly { return default; }

Another thing that is allowed but has a hole IMHO

public static T M<T>(in T t){ return default; }

Specifically

public static T M<T>(in T t) where T : struct { return default; }

Uninteresting code

public static T M<T>(in T t) where T : struct { return default; }
    public void M() 
    {
        var m = M<int>(default);
        Console.WriteLine(m);//0 (0x00000000)
    }

I propose that the readonly generic constrain be created such that if the type is not readonly or passed by in that the compiler should throw a warning CSXXX otherwise should compile as allowed.

The spec should be modified if required such that if a method in which return default is used in the aforementioned scenario and the param is not passed by in or declared readonly then the result should be null otherwise the in value and not another new default value

Motivation memory protection semantics can be built around this inter alia

To be combined with the other proposed or existing constrains such as but not limited to struct or interface

Possibly make readonly available as a method keyword everywhere.

@juliusfriedman juliusfriedman changed the title readonly interface constraint readonly generic constraint Jul 16, 2020
@yaakov-h
Copy link
Member

why is returning default from a method a "hole" in in?

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

because we allocate stack space and initialize on the stack for the supposed given already allocated value.

@yaakov-h
Copy link
Member

... would you care to elaborate? That still doesn't make sense.

@juliusfriedman
Copy link
Author

What does't that we do something we should not?

@canton7
Copy link

canton7 commented Jul 16, 2020

I can see an argument for allowing:

public interface IS { void Foo(); }
public readonly struct S { public void Foo() { } }

public void M<T>(in T s) where T : readonly, IS
{
    s.Foo(); // Doesn't make a defensive copy of 's'
}

However:

  1. I suspect readonly members on interfaces #3055 is a better way of doing this, and
  2. That doesn't seem to be what this proposal is about anyway

In fact, I'm having a very hard time trying to understand what's being proposed. Let me walk through my confusion:

if a method in which return default is used in the aforementioned scenario

OK, so we've got a generic method with a where T : readonly constraint. Presumably this implies where T : struct, since only structs can be readonly?

and the param is not passed by in or declared readonly

Is this meant to parse as "and the param is (not passed by in) or (declared readonly)" or should it be "and the param is not ((passed by in) or (declared readonly))"? Params can't be "declared readonly" anyway, so I'm going to ignore that bit for now and read it as "and the param is not passed by in".

OK, so we're looking at a method like:

public T M<T>(T t) where T : struct, readonly
{
    return default;
}

then the result should be null

Wait, what? T has to be a struct. Structs can't be null.

otherwise the in value and not another new default value

What does "the result should be null otherwise the in value" mean? What determines whether default produces null or "the in value"? What even is "the in value", since we're describing a method which doesn't have any in parameters?

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

I am good with #3055 doing what it does there, here is different.

Do not 0 init the stack, it should be initialed already for the struct already by the caller. no return other that t should be made due to the readonly constraint.
If readonly in a constraint must have other meaning I propose readonly as a method keyword similar to static

@canton7
Copy link

canton7 commented Jul 16, 2020

What does readonly have to do with returning values from a method?

C# already has readonly methods, they were introduced before readonly struct.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

What does readonly have to do with returning values from a method?

C# already has readonly methods, they were introduced before readonly struct.

Where does one read about the semantics of readonly methods tbh not that familiar.

@canton7
Copy link

canton7 commented Jul 16, 2020

A readonly struct is just a struct where all members are readonly (it enforces that all fields are readonly, and implicitly makes all methods readonly). See here

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

A readonly struct is just a struct where all members are readonly (it enforces that all fields are readonly, and implicitly makes all methods readonly). See here

I know about readonly structs, I need to learn about readonly methods. I cannot find any ref to this in the docs you linked can you cite the ecma spec part?

The readonly modifier cannot be applied to static methods, constructors or destructors. doesn;t help

public readonly int M() {
        return 1;
    }

This is not valid...

@mikernet
Copy link

mikernet commented Jul 16, 2020

no return other that t should be made due to the readonly constraint.

So are you saying that return default would actually return t, as in...

public static T M<T>(in T t) where T : readonly { return default; }

var m = M<int>(5);
Console.WriteLine(m);

...would actually print 5?

@mikernet
Copy link

From what I understand you're saying you don't want the in T t value to be copied and returned because it is already on the caller's stack?

@canton7
Copy link

canton7 commented Jul 16, 2020

I linked the spec proposal -- that's the best we have. We only have an ECMA spec for C# 5.

The "Motivation" section lays it out. This doc talks about it a bit as well.

readonly structs and methods are used with the in modifier. in passes in the struct by reference, but it also promises that the copy of the struct owned by the caller won't be modified by the method. In order to ensure this, the compiler would normally insert defensive copies of the struct whenever one of its fields is changed, or one of its methods is called (because the compiler doesn't know whether a method might mutate the struct), so that any mutations happen to a copy, not the original held by the caller.

For example:

public class C
{
    public void Test()
    {
        S original = new S();
        M(in original);
    }
    
    public void M(in S s)
    {
        s.NonReadonly();
    }
}

public struct S
{
    public void NonReadonly() { }
}

SharpLab

The compiler inserts a hidden copy of s just before calling NonReadonly(), so that if NonReadonly mutates the struct, then original isn't changed.

If you give the struct a readonly method, then the compiler ensures that it cannot mutate the struct. This means that it's free to call the method without making a hidden copy:

public class C
{
    public void Test()
    {
        S original = new S();
        M(in original);
    }
    
    public void M(in S s)
    {
        s.Readonly();
    }
}

public struct S
{
    public readonly void Readonly() { }
}

SharpLab

There's no hidden copy of s here.

A readonly struct will make all of the struct's methods readonly (and ensures that all fields are readonly), so you don't need to put readonly on each and every method.


As you can see, none of this is to do with returning values from a method, it's all about whether or not a defensive copy of a struct is made before calling one of its methods.

This is why I'm confused by your focus on return values.

@mikernet
Copy link

mikernet commented Jul 16, 2020

From what I understand you're saying you don't want the in T t value to be copied and returned because it is already on the caller's stack?

Follow-up: is it possible that during inlining the JIT elides this? I don't know, but I think it could.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

Ewww this is on structs only? why?

@canton7
Copy link

canton7 commented Jul 16, 2020

... because you can have readonly struct but not readonly class, and can't have readonly methods on a class? Because the entire point of readonly types is around in and passing structs by reference?

You're the one talking about readonly types! You must know that you can have a readonly struct but not a readonly class.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

... because you can have readonly struct but not readonly class, and can't have readonly methods on a class? Because the entire point of readonly types is around in and passing structs by reference?

You miss the point, I have a struct I pass by in, i get that in from a ref..

@canton7
Copy link

canton7 commented Jul 16, 2020

Then why did you say

Ewww this is on structs only? why?

?

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

Then why did you say

Ewww this is on structs only? why?

?

Because why, any method should be able to designate readonly on return not just struct and yes I can make a wrapper type but now that can be subverted

@HaloFour
Copy link
Contributor

readonly structs+methods exist to inform the compiler to not make defensive copies of the struct for the sake of guarding against in-situ modification. Classes can't be defensively copied. The in/readonly ref is just a ref where the developer is promising that they are not attempting to modify the struct.

The T passed into the generic method by-ref has nothing to do with the value returned from the method, so I don't really understand what this proposal is attempting to accomplish. Returning anything other than default(T) would be wrong. Even returning a ref T of the in T would be wrong as that breaks the promise that the struct cannot be modified as the callee cannot make any guarantees about the caller. At best you'd need in returns to extend that promise to the caller.

@canton7
Copy link

canton7 commented Jul 16, 2020

Because why, any method should be able to designate readonly on return.

What do you mean by "on return"? readonly has nothing to do with returns.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

readonly structs+methods exist to inform the compiler to not make defensive copies of the struct for the sake of guarding against in-situ modification. Classes can't be defensively copied. The in/readonly ref is just a ref where the developer is promising that they are not attempting to modify the struct.

The T passed into the generic method by-ref has nothing to do with the value returned from the method, so I don't really understand what this proposal is attempting to accomplish. Returning anything other than default(T) would be wrong. Even returning a ref T of the in T would be wrong as that breaks the promise that the struct cannot be modified as the callee cannot make any guarantees about the caller. At best you'd need in returns to extend that promise to the caller.

I like readonly structs.

You could return nullref

@juliusfriedman
Copy link
Author

Because why, any method should be able to designate readonly on return.

What do you mean by "on return"? readonly has nothing to do with returns.

Perhaps it should in some cases where I return the in

@HaloFour
Copy link
Contributor

Ah, so you want something like this:

    public readonly ref T M<T>(in T v) where T : struct {
        return ref v;
    }

@canton7
Copy link

canton7 commented Jul 16, 2020

I'm trying to get you to explain how you think readonly relates to returns, because I and others are really struggling to understand what you want

@juliusfriedman
Copy link
Author

Ah, so you want something like this:

    public readonly ref T M<T>(in T v) where T : struct {
        return ref v;
    }

I could palette that.

@HaloFour
Copy link
Contributor

Nod, this interrogation/negotiation style of "proposal" is exhausting.

@canton7
Copy link

canton7 commented Jul 16, 2020

I considered readonly ref, but none of this examples in the OP have ref in them, so I assumed that wasn't what they were asking about.

@mikernet
Copy link

mikernet commented Jul 16, 2020

Is this kinda what you want?

public static in T M<T>(in T t) where T : struct { return t; }

Where the return value must be an in T value so that the in ref can just be returned without a copy?

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

Yes and what @HaloFour also showed are kinda of in the same dimension I think AFG

@juliusfriedman
Copy link
Author

I considered readonly ref, but none of this examples in the OP have ref in them, so I assumed that wasn't what they were asking about.

I can add them it just makes it harder or worse imho

@canton7
Copy link

canton7 commented Jul 16, 2020

It can't be more confusing than what you have already 😈

Is it correct that you want to be able to write something like this:

public ref T M<T>(ref T v) where T : struct {
    return ref v;
}

But where the parameter is in, not ref?

@mikernet
Copy link

I think AFG

What's AFG? I haven't heard that one before. Sorry if I'm just illiterate with internet speak

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

I like that example

public ref T M<T>(ref T v) where T : struct {
    return ref v;
}

and will say yes but I really like @mikernet example also

public static in T M<T>(in T t) where T : struct { return t; }

Although I would prefer to read it as

public static readonly T M<T>(in T t) where T : struct { return t; }

@juliusfriedman
Copy link
Author

I think AFG

What's AFG? I haven't heard that one before. Sorry if I'm just illiterate with internet speak

At first glance, sorry

@canton7
Copy link

canton7 commented Jul 16, 2020

public static readonly T M<T>(in T t) where T : struct { return t; }

The problem here is that you're returning a pointer to a struct, but readonly does not in any way imply a pointer. ref, in and out imply pointers.

This method is a readonly method (which I defined above) that returns a struct by value. If you remove static and put it in a struct, it is legal today


public ref T M<T>(ref T v) where T : struct {
    return ref v;
}

This is legal today, if you hadn't realised.

@HaloFour
Copy link
Contributor

Initially the syntax for in was proposed to be readonly ref. It was changed to in to make it more ergonomic despite being a somewhat niche feature, a decision that still riles some individuals on this repo. readonly ref makes sense here as it describes what it is, a ref that is readonly.

@canton7
Copy link

canton7 commented Jul 16, 2020

Sure, readonly ref T M<T>() is clearish. That's different to readonly T M<T>() which means something entirely different.

@juliusfriedman
Copy link
Author

Sure, readonly ref T M<T>() is clearish. That's different to readonly T M<T>() which means something entirely different.

That;s why I like @mikernet's example.

readonly ref is something different but plausible.

@canton7
Copy link

canton7 commented Jul 16, 2020

I would recommend updating your OP, and clarifying that you want to be able to return a reference to a struct from a method, and allow that reference to be one that was passed into the method using in. Nothing to do with readonly generic type constraints or default.

Make sure you also include motivating examples -- sure you could do this, but why would you want to, for instance.

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 16, 2020

Will do asap, ty for your assistance and critique

@333fred
Copy link
Member

333fred commented Jul 16, 2020

I would recommend updating your OP, and clarifying that you want to be able to return a reference to a struct from a method, and allow that reference to be one that was passed into the method using in. Nothing to do with readonly generic type constraints or default.

To be clear, that's already possible: https://sharplab.io/#v2:CYLg1APgAgTAjAWAFBQMwAJboMLIN7LpHqHFroBOApgGaVUCGwA9gHYA2AnugCroCyAHh4A+ABQBLVr3QA3AJToA7gAsq1GSHQBnAC4UArgGNd6AkmKXMAdnp1ZAblJEAvshdA==

@juliusfriedman we appreciate the enthusiasm. But your proposals need more work. You need to understand the spec and propose actual changes that don't conflict with the existing language. Afaict, this has been 30+ messages to understand that you are trying to propose something that already exists.

@HaloFour
Copy link
Contributor

D'oh, didn't realize it was ref readonly and that it was already added to the language! :D

@mikernet
Copy link

🤣 Haha, whoooops.

@CyrusNajmabadi
Copy link
Member

@juliusfriedman we appreciate the enthusiasm. But your proposals need more work. You need to understand the spec and propose actual changes that don't conflict with the existing language. Afaict, this has been 30+ messages to understand that you are trying to propose something that already exists.

I agree with Fred here. And this ties into the conversation you and I were having yesterday on emphasizing clarity and providing more context and information to assist with the discussion. Being vague and responding cryptically does not help your proposals out.

Given taht this appears to be a proposal for something that already exists, I'm goign to close this out. You are more than welcome to continue making proposals here. My only request is that you invest a little more time into them to help save a lot of people spending many hours trying to unpack exactly what it is you are referring to.

Thanks much!

@juliusfriedman
Copy link
Author

juliusfriedman commented Jul 17, 2020

No prob so long as you do the diligence you promised and we reopen when appropriate and if warranted.

otherwise i find this very faux paus`

@mikernet
Copy link

Why would this be reopened? The exact feature you are proposing already exists, the syntax for it is above.

@juliusfriedman
Copy link
Author

Why would this be reopened? The exact feature you are proposing already exists, the syntax for it is above.

I said when and if...

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

7 participants