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

[Proposal] Readonly method parameter as a way to optimize value passing #3202

Closed
KalitaAlexey opened this issue May 31, 2015 · 16 comments
Closed

Comments

@KalitaAlexey
Copy link

Let's consider method

void Method(SomeStruct arg)

On call arg contains copy of passed argument.
Let's consider adding modifier readonly to parameter.

void Method(readonly SomeStruct arg)

Now arg may contain reference
Like it was called

void Method(ref SomeStruct arg)

SomeStruct someStructInstance = <...>;
Method(ref someStructInstance);

It is a optimization and it is preventing misunderstanding methods which accept ref parameters just for not copying

I determine some rules those method must fit:

  • It cannot assign any properties of a readonly parameter
  • It cannot assign readonly parameter to anywhere outside of function.
  • It cannot pass this parameter to methods as not-readonly parameter
@Joe4evr
Copy link

Joe4evr commented May 31, 2015

Already listed in #2136, making this a dupe of #115.

@HaloFour
Copy link

The one really big loophole with this is that value types can have self-mutating methods, which look and are invoked just like any other normal method. If the variable is a reference to a value type invoking such a method would modify the value type.

static void Foo(readonly Point pt) {  // ref is implied, per the proposed syntax above
    pt.Offset(2, 2);  // C# compiler has no idea that this method internally mutates the Point
}

static void Main() {
    var pt = new Point(2, 2);
    Foo(ref pt);
    // oops, pt is now {4, 4}
} 

@whoisj
Copy link

whoisj commented May 31, 2015

@HaloFour I do think the this reference methods receive when executing would need to acquire a transitive readonly attribute. Being transitive, the readonly would apply to all members of this thus causing a compilation error -- assuming the compiler could safely know that the methods did or did not modify the object.

To make this work, we may need the concept of const methods, or methods that do not modify the object they are associated with.

@HaloFour
Copy link

@whoisj The CLR has no concept of those things and they would represent breaking changes. As of now, a value type can define mutating methods and to the caller they look like any other value type instance method. The C# compiler has no idea that Point.Offset happens to change the value, the only way it could figure that out would be to decompile the method (recursively) to try to determine that fact. The IL of the Foo method above is simply:

ldarg.0
ldc.i4.2
ldc.i4.2
call instance void [System.Drawing]System.Drawing.Point::Offset(int32, int32)
ret

@whoisj
Copy link

whoisj commented May 31, 2015

@HaloFour hence my suggestion about const methods.

@HaloFour
Copy link

HaloFour commented Jun 1, 2015

@whoisj Which is an interesting idea but in order to actually work would require implementation and enforcement at the CLR level, as well as retrofitting across the entire BCL.

@whoisj
Copy link

whoisj commented Jun 1, 2015

@HaloFour would it not be just a new CLR feature?

@Joe4evr
Copy link

Joe4evr commented Jun 1, 2015

@HaloFour

The C# compiler has no idea that Point.Offset happens to change the value

It could easily test this without decompiling, could it not?

Assert.AreEqual(default(Point).Offset(2, 2), default(Point)); //or whatever the testing framework that ships within VS uses

@HaloFour
Copy link

HaloFour commented Jun 1, 2015

Adding runtime checks to assert would require making a copy of the value
type to compare and completely nullify any performance benefit of using
ref.

On Mon, Jun 1, 2015, 3:41 PM Joe4evr notifications@github.com wrote:

@HaloFour https://github.com/HaloFour

The C# compiler has no idea that Point.Offset happens to change the value

It could easily test this without decompiling, could it not?

Assert.AreEqual(default(Point).Offset(2, 2), default(Point)); //or whatever the testing framework that ships within VS uses


Reply to this email directly or view it on GitHub
#3202 (comment).

@KalitaAlexey
Copy link
Author

Rule is going to help
•It cannot pass this parameter to methods as not-readonly parameter

@Joe4evr
Copy link

Joe4evr commented Jun 2, 2015

Who said anything about doing the checking at runtime? I meant it should run the check in the background during design/compile time, and emit an error if it failed.

@KalitaAlexey
Copy link
Author

@Joe4evr Do you suggest call function and then compare original object and object after method call?
It will lead to bugs with very little possibility, when method behavior depends not just on parameter but also on another external or internal factors.

@Joe4evr
Copy link

Joe4evr commented Jun 2, 2015

@KalitaAlexey Something like that. Though what kinds of factors would alter how a method behaves, exactly?

@KalitaAlexey
Copy link
Author

@Joe4evr I am wrong. We don't need any checks, because if parameter passes to a method as not readonly, then this parameter will be copied as before

struct SomeStruct
{

}


class Program
{
    static void MethodAcceptsReadonlyParameter(/*readonly*/ SomeStruct someStructInstance)
    {
        // Here method accepts copy of someStructInstance
        MethodAcceptsCommonParameter(someStructInstance);
    }

    static void MethodAcceptsCommonParameter(SomeStruct someStructInstance)
    {

    }

    static void Main(string[] args)
    {
        var someStructInstance = new SomeStruct();
        // Here method accepts reference to a someStructReference
        MethodAcceptsReadonlyParameter(someStructInstance);
    }
}

@HaloFour
Copy link

HaloFour commented Jun 2, 2015

@Joe4evr Through what mechanism? The compiler doesn't execute your code as it is being compiled, and even if it could it could only do so in the simplest of scenarios. Short of CLR-enforced "pure" methods there is no way to guard against the struct being modified by its own method.

@gafter
Copy link
Member

gafter commented Mar 20, 2017

We are now taking language feature discussion on https://github.com/dotnet/csharplang for C# specific issues, https://github.com/dotnet/vblang for VB-specific features, and https://github.com/dotnet/csharplang for features that affect both languages.

See also #18002 for a general discussion of closing and/or moving language issues out of this repo.

We are tracking this particular request at dotnet/csharplang#188

@gafter gafter closed this as completed Mar 20, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants