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: Add support for appending ! to method names to denote method mutates object passed in. #2696

Closed
OmarElabd opened this issue May 12, 2015 · 28 comments
Labels
Area-Language Design Discussion Resolution-External The behavior lies outside the functionality covered by this repository

Comments

@OmarElabd
Copy link
Contributor

This comes straight out from ruby conventions for denoting methods with side affects. That is that the method will modify the object is it called on.

Consider the two following methods which double the size of a square, the first one is "dangerous" as it mutates the object that is passed in.

public Square Enlarge!(Square square)
{
   square.side *=  2;

    return square;
}

The second method does not make any changes to the original object and is "safer".

public Square Enlarge(Square square)
{
   return new Square(square.side*2)
}

The closest current equivalent is the [Pure] attribute in the code contracts (although it denotes the opposite). However there are numerous disadvantages with using that attribute:

  1. Required code contracts
  2. Not obvious to the code caller that the method is pure (they would need to look at the actual source code to observe that it is indeed "safe").
  3. It is more useful to indicate a potentially dangerous method rather than a safe one.
@paulomorgado
Copy link

!like

@dsaf
Copy link

dsaf commented May 13, 2015

Practically, here and today, you can just use the JetBrains's one, which will give you a proper IDE support: http://www.jetbrains.com/resharper/webhelp80/Reference__Code_Annotation_Attributes.html#PureAttribute

Even Entity Framework 7 team is using those:

https://github.com/aspnet/EntityFramework/search?q=jetbrains

Besides that:

  1. Have you seen the language-level contracts proposal Proposal: Method Contracts #119?
  2. Is it simply a declaration or a compiler-enforced guarantee? The latter is much more useful but hard to implement - traversing all the potential method chains etc.
  3. I am personally not a fan of ! syntax. It is already considered for non-nullability.

@paulomorgado
Copy link

How would delegates work? Wold they require that ! too? What would this code mean then?

d1!=d2

Is it a boolean expression or a statement?

Can I assign a non ! delegate with a ! method?

What about overloads? Is Method! and overload of Method, or does it belong to another method group?

Is this ! mainly cosmetic (like prefixing fields with _) or does it have a meaning in the language?

@OmarElabd
Copy link
Contributor Author

@paulomorgado it doesn't need to necessarily be a ! it could be anything, just some indication that a method is dangerous. I would think that they would infact be overloaded in the scenario you mentioned. I would think it would be cosmetic/conventional but could be worked into things such as CodeContracts and tools like ReSharper could provide hints to the user as well.

@HaloFour
Copy link

Considering that "not-safe" or "not-pure" is the default today I think changing the syntax required to specify that default is a bit of a non-starter. It would require that the majority of code be modified in order to support this syntax, even if it were opt-in.

How does this not have the same problem that decorating a method with PureAttribute doesn't? It would have to be encoded as an attribute anyway since the CLR has no concept of "pure" or "safe" methods and as such it can be easily spoofed by any compiler that doesn't enforce analysis of the compiled code. An ILDASM/ILASM roundtrip could easily mark methods as safe when they mutate any object they want and the compiler would be none-the-wiser.

I think that a good analyzer that can verify methods marked as Pure is the way to go. True, it can't verify that some code in an assembly is pure, but neither could this compiler syntax.

@OmarElabd
Copy link
Contributor Author

@HaloFour I'm proposing that it explicitly not be implemented as an attribute as that has issues (i mention them in the original post). Calling an Expand!() method tells every caller that the method can cause side effects to object your passing in. This does not need to be enforced by the compiler, it can just be a convention for users.

As a user it is extremely useful for me to know that the method i'm calling will modify my object, instead of needing to experiment to find out how the method will act.

@HaloFour
Copy link

So really this proposal is to permit the use of the ! character in an identifier and no behavior is implied. Why not just select a convention based on already legal identifier characters?

@OmarElabd
Copy link
Contributor Author

@HaloFour Ruby already has this convention, i think it makes more sense to adopt it directly from Ruby as a lot of people can infer what it is implied by the !.

And eventually perhaps compiler behaviour can be added to support it, not only as a convention but maybe someday as a feature.

@HaloFour
Copy link

I'd rather a C-family convention, which would be marking "pure" methods as "pure" and not vice-versa. Attributes are already provided for this effect. Ruby is a very different language syntactically and behaviorally. I doubt that there is all that much cross-over and I doubt that most C# programmers would understand this ! convention. Changing how it behaves after-the-fact would also represent a breaking change. Again, if this is just a naming convention, why not choose an already-legal name? No need to change the spec and parser behavior to denote methods you want to be unsafe.

@OmarElabd
Copy link
Contributor Author

@HaloFour An attribute doesn't tell me anything about the code i'm about to call unless i look at the source code, which may involve decompiling and inspecting the calling method it's compiled into a dll.
A symbol is far more appropriate for the following two reasons:

  1. It is in the method name so anyone can see it (no need to decompile dlls to inspect attributes)
  2. A symbol is more useful than some legal name convention, I don't believe any single name would be adopted, as that would break backwards compatibility. Imagine if the keyword was something like ExpandUnsafe(), how much existing code has a "Unsafe" keyword appended to it? The answer to that is ???. Now how many users have "!" appended to their methods? 0. It wouldn't break anyone's existing code.

@HaloFour
Copy link

  1. When do you ever have to decompile an assembly in order to inspect the attributes? They are available front-and-center via reflection and very easily available to any IDE which can denote these methods differently. A name would certainly be more easy to read from a plain-text editor with no reflective/completion capabilities.
  2. A symbol might be more useful, but it's still just a name. As it serves no actual purpose to the compiler or run-time forcing a specific symbol is pointless, especially one that requires changing the identifier rules for the language. The ! symbol already has meaning in the C# language which is unrelated to method purity. Also, the ! symbol violates CLS naming conventions. Also, attempting to standardize on a new naming convention, even with a legal suffix, would only be massively confusing given that none of the CLR follows said convention so the vast majority of mutating code would be undecorated.

@HaloFour
Copy link

Anywho I think we'll just have to agree to disagree. I do agree that there is value in being able to easily identify methods that are pure/mutating as well as have the compiler/analyzers provide better diagnostics around them.

@OmarElabd
Copy link
Contributor Author

@HaloFour I appreciate you bringing up those points, it's always good to get a look at the other side of things.

@paulomorgado
Copy link

@OmarElabd, you can always use _ instead of ! in your code if that pattern fits your preference.

@dsaf
Copy link

dsaf commented May 13, 2015

@HaloFour @OmarElabd

I now think you are both right, in a way that the new syntax is not needed but the attributes are not visible to consuming developers in any useful manner.

I have already asked the Roslyn team to address this unfortunate problem, but they are reluctant. As if they see attributes as some necessary evil that must be swept under the carpet #711.

@HaloFour

...very easily available to any IDE which can denote these methods differently...

Could you then please list all those wonderful IDEs? Unless I am doing something wrong the only way to see attributes is by going to sources. The only random exception is made for the ObsoleteAttribute for an arbitrary reason #781, and even then you have to hover over for a tooltip.

Last time I checked the only way @OmarElabd can get something close to what he wants is by using Visual Studio in conjunction with ReSharper and the properly configured Enhanced Tool-tip plugin: https://github.com/MrJul/ReSharper.EnhancedTooltip

@HaloFour
Copy link

@dsaf

You're right that VS doesn't provide any visual indication of pure methods or for arbitrary attributes today. You'd need an extension which should be fairly trivial to write.

If/when the Roslyn team does decide to pick up proper support for "pure" methods I'd hope that they would consider ways to identify them better in the IDE.

@OmarElabd
Copy link
Contributor Author

@HaloFour
that's not entirely accurate though. Not only would you need to write an extension for Visual Studio. If you really wanted to support it you'd need to write it for Visual Studio Code, Xamarin's IDE, Notepad++, Sublime, etc...

@HaloFour
Copy link

@OmarElabd

Sure, but they each have to do their own thing as it is to support basic syntax highlighting and code completion. Indeed it's not as obvious as something like a naming convention.

Is a naming convention applied this late in the game really any better, though? Especially one where the undecorated methods are assumed to be "safe/pure"? The vast, vast majority of methods called aren't going to be decorated with this naming convention, or be safe/pure, which would be confusing to any developer on that project. If I was going that route in my projects I would likely adopt a convention to mark the pure methods instead.

@OmarElabd
Copy link
Contributor Author

@HaloFour Why not both? '!' for unsafe '*' for safe? That way the lack of an operator implies nothing, but the presence of one immediately tells you something.

@HaloFour
Copy link

@OmarElabd Why not Pure and Unpure or P and U, which are perfectly valid today? Why does it have to involve changing the C# spec to include characters explicitly forbidden in the CLS naming guidelines due to potential parsing problems with other CLR languages?

@OmarElabd
Copy link
Contributor Author

@HaloFour the biggest issue is we do not want to mistake previously written libraries and code as pure/unpure. An existing method called RetrievePure() would be misconstrued as a pure method when it is not because Pure in this case (that is in the RetrievePure() method) is actually an acronym or something else. Whereas because special characters have never been allowed previously in method names this issue does not exist with special characters.

@HaloFour
Copy link

@OmarElabd But * and ! wouldn't mean "pure" and "not pure", at least where the compiler is concerned. They would just be normal identifiers like any other that anyone could use/abuse for any purpose and despite what little convention is already set by the BCL. It would also likely produce bizarre parsing problems in various languages since they have no reason to expect a method to contain those characters since the CLS explicitly forbids them.

@AdamSpeight2008
Copy link
Contributor

It's going to get confusing since there are already proposal in which '!` denotes non-null

@whoisj
Copy link

whoisj commented May 21, 2015

I find myself agreeing with @HaloFour. C# has a long history of assuming mutability and impurity. Asking developers to change their mental model to meet parity with Ruby isn't going to go over well. I would expect a mass rejection.

However, C++ already has a model that works well that C# should follow. Why not invert this discussion and mark methods that do not modify their object using the const keyword.

public const Square GetDoubleSized()
{
    return new Square(this.Size * 2); // legal, does not modify this 
}

public const Square Enlarge()
{
    this.Size *= 2; // illegal, modifies this
    return this;
}

The concept could be expanded to parameters as well (like the original posted is asking for):

public const Square GetDoubleSized(const Square square)
{
    return new Square(square.Size * 2); // legal, does not modify this or square
}

public const Square GetDoubleSized(const Square square)
{
    square.Size *= 2; // illegal, modifies square
    return square;
}

@OmarElabd
Copy link
Contributor Author

@whoisj Actually that sounds like a really good idea, const could be used for methods that do not modify methods and something like 'volatile' could be used to indicate the opposite. There would need to be some sort of indication from intellisense that a method is marked as 'const'.

@sharwell
Copy link
Member

You're right that VS doesn't provide any visual indication of pure methods or for arbitrary attributes today. You'd need an extension which should be fairly trivial to write.

This is one of the fundamental features of the Code Contracts Editor Extensions extension, which is under active development.

@JoergWMittag
Copy link

This comes straight out from ruby conventions for denoting methods with side affects. That is that the method will modify the object is it called on.

That is not what the ! means in Ruby method names.

First off, although it has already been stated, I want to repeat that, in Ruby, there are no semantics associated with !. It is simply a valid identifier character as the last character of an identifier. That's it.

It is also not about side-effects. There are plenty of side-effecting methods that don't have a ! in their name, e.g. print.

There are also plenty of mutating methods without ! in their name, e.g. just looking at the methods of String, there are clear, concat, force_encoding, insert, prepend, replace, setbyte, <<, and []=, all of which modify the receiver, and none of which have a ! in their name.

OTOH, there are methods like exit!, which do have a ! in their name, but don't modify the receiver. exit! exits the current Ruby process without running exit handlers.

What the ! convention (and it is just that, a convention) really means is that if there are two methods which do the same thing in two different ways, then the one which does it the more surprising way gets the !.

So, ! does not mean "side-effect", it does not mean "mutate the receiver", and it doesn't get applied to all methods that fit the criteria. ! means "surprise", and it only gets applied when there is a corresponding non-! method.

In Scheme, ! indicates solely that a procedure mutates one or more of its arguments. There does not have to be a non-mutating counterpart. ! does not get used for side-effects or "surprises" other than mutating an argument.

In Reia, ! is part of the language semantics, but OTOH, it is not part of the method name. In Reia, appending ! to a method call updates the local variable with the result of the method call, IOW, it is syntactic sugar which turns

foo.bar!

into

foo = foo.bar

Note that this is different from mutating foo. bar might return an updated immutable copy of foo, for example.

@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.

@gafter gafter closed this as completed Mar 20, 2017
@sharwell sharwell added the Resolution-External The behavior lies outside the functionality covered by this repository label Jun 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Language Design Discussion Resolution-External The behavior lies outside the functionality covered by this repository
Projects
None yet
Development

No branches or pull requests

9 participants