-
Notifications
You must be signed in to change notification settings - Fork 4k
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
Allow ref-returning get-only auto-properties #15623
Comments
I hope this is some kind of joke. |
There is nothing "modern" about micro-optimisation. Did you mean "expression-bodied"? Modern is going to be like this #5172 : var p = new ModernPerson(...);
var q = p with { Age = p.Age + 1 }; PS: I personally hate exposed default mutability - not setters. |
@mikedn how is this a joke???!? Jared Parsons claims passing properties by ref is a highly requested user scenario: https://twitter.com/jaredpar/status/804070933954633728 I believe him! And it is now possible to express such modern properties in C# 7.0! The only thing I want is to reduce boilerplate and extend auto-property syntax just a tiny bit. |
It might not work with e.g. WPF bindings (if that's a consideration). |
@controlflow I'd agree with @dsaf. If you are going to expose it like that, there is no reason to use an auto-property (at least not something that I can think of). |
There is nothing "modern" about misusing This misuse of |
@alrz no reason? How about polymorphism? I can implement interface or override base property. Also a lot of binding/serialization frameworks require properties to be used in user types. |
@DavidArno how do you define "misuse"? Why ref-returning methods and assigning to method invocations is somehow fine and beneficial to everybody, while ref-returning properties are not??? |
You're taking things out of context. The number of scenarios where
Considering your replies in that Twitter conversation I take it that this is indeed a joke.
There's nothing modern about exposing a private field via a
I don't think there are enough useful scenarios for that. This "modern property" thing is certainly not an useful scenario. |
Exhibit (a): person.Age ++; // no setter call!! This is a downright offensive misuse of ref returns. |
@dsaf AFIAK |
Indeed, what's this supposed to mean? The JIT usually inlines trivial property getters and setters so there's no call anyway. How can this be a serious justification for |
what is the difference between person.Age ++; and a Span<int> s = ...;
s[42] ++; ? I don't think that |
@mikedn I'm not justifying the existence of ref-returning properties, they are already in a language! I'm so happy about this, just asking about corresponding auto-property syntax. |
You're trying to justify extending the support for |
And this is exhibit (b). You are again misusing ref returns. They aren't intended to allow you to start modifying such data. Their purpose is to allow optimised returns that expose lots of value data without having to duplicate that data when returning. Using ref returns to modify that data is a misuse of this feature. |
@DavidArno As long as we don't have |
@DavidArno ref returned reference is mutable and |
@DavidArno you should've guessed by now that the design team holds a neutral position on the question of mutability and will not make things immutable when they can be left mutable. Properties can already be made ref-gettable (I actually wonder how much faster |
Now let's not get into the opposite extreme. There's nothing wrong with modifying data using ref returns, it's only the abuse of this feature that could be wrong. The reality is that this feature should have been accompanied by It's likely that if this "modern property" thing wasn't used here then few people would have commented on this issue. The feature request itself isn't wrong (though it seems rather useless to me) but the feature justification is just weird. |
The |
@alrz and @orthoxerox, You are both, of course, correct. I still get frustrated though that their "neutral position on the question of mutability" position inevitably leads to things like this request. At least @dsaf knows how to write good code, and hopefully soon: var p = new ModernPerson(...);
var q = p with { Age = p.Age + 1 }; really will become the modern way of writing code 🎉 |
Come to think of it, there's one case I know of where a ref property might be useful: interpolation/tweening in video games. There are lots of cases where you have to interpolate some arbitrary value (camera shake, menu panel speed) and this is commonly done by a separate tweener that is called by the game loop. You call a method like |
@mikedn How would one lose index validation? Before you return the |
For example: l.Add(42);
ref int x = ref l[0];
l.RemoveAt(0);
x++; It's actually worse: l.Add(42);
ref int x = ref l[0];
l.AddRange(Enumerable.Range(1, 10000));
x++;
|
Without ownership and immutability it's really easy to shoot yourself in the foot with fn main() {
let mut l = vec![42];
let x = &mut l[0];
l.remove(0);
*x += 1;
} I'm just trying to say that throwing a |
In C# 10 we might need unique_ref, shared_ref abstractions similar to C++, go figure. 😆 |
@mikedn Yes, and that's what will happen when you replace the list with an array as well. |
@eyalsk I just don't know what's the rush to ship ref locals before readonly locals. i.e. as soon as possible! Before doing something about immutability, I think that's just wrong. Perhaps if you ask, "it's highly requested feature" is the best answer that you might get. And we all know how customers request features. |
I was curious about this. Turns out there is a difference in performance between the two, but it's fairly small (depending on your point of view). First, decompiled assembly: "OldSchool": mov edx, dword ptr [rcx+16]
inc edx
mov dword ptr [rcx+16], edx "Modern": mov r8, rdx
inc dword ptr [r8] Benchmark results (using x64 .Net Framework):
So for my benchmark, the difference is about 7 %, which doesn't sound like that much to me, considering what you lose by switching to such "modern" style. |
The only reason why those 2 versions don't generate identical code is a rather unfortunate present limitation of the JIT. That limitation will have to be removed sooner or later (there are already multiple issues opened for that) because it affects many other things, not just this particular use of properties. In fact, even the It's also possible for the ref version to generate code that's potentially worse, for example if after |
First:
This type of language is just not necessary. These are tools for solving problems. Second, I believe 'ref' to be a fairly advanced feature intended explicitly for driving performance in constrained scenarios. The customers requesting this feature do not want 'ref' purely to get readonly data back. They want 'ref' returns so they can easily get access to and mutate values in their system. One of the common cases for this is a game engine that has large numbers of value types representing pieces of their system. Then, may times a second, they go an update/mutate these values. These individual values may be so large that passing them around is actually a perf issue. So passing them by-ref is a valuable savings. They get the benefits of:
-- Mutation of ref-values is not viewed as a negative. It's viewed as a core reason why we did the feature in the first place. |
There was no rush on this. It was simply easy to do, and solved a need for several customers. The difference between the two features is that without ref-returns, some types of programs simply weren't possible for customers. i.e. the perf overhead of existing C# constructs was always too high for them to get the performance they needed. Readonly-locals is not the same. The lack of such a feature doesn't make some programs impossible to write. It simply makes the language less pleasant for those that want as much immutability as possible.
I'm sorry you think that. But we don't feel like it's appropriate to hold up significant blocking issues in the language, when we think we have perfectly suitable solutions, for other features which are nice-to-have, but are not blocking anyone.
I'm happy to discuss every decision we make. This was an easy one:
If it helps, Jared was heavily involved in tihs, and he comes to this feature with a tremendous amount of experience doing work in this area for Midori. He's well versed in all the important issues around ref, readonly, readonly-ref, immutability, etc. He played a large part in being able to do this small, beneficial, sub-feature, while still ensuring that we could do future work here as we moved forward. |
Indeed, these are tools. So is a shotgun. But a shotgun makes a lousy bottle opener.
I agree. As an advanced tool I also think that it should also come with a warning label. Otherwise, if Not a knock on |
I was simply pointing out that it was a core design goal to enable mutation with refs. That was a primary scenario that customers need for the problems we were trying to address. So stating that it's a 'misuse' to allow for mutation of ref-returns does not actually fit with our design goals for the feature. A core intent is to allow cheap mutation of large value types. We would certainly like features like 'readonly-ref' in the future. But in the short term a primary goal was precisely to enable exactly this behavior. |
7% sounds absolutely massive to me. We're happy when we introduce a compiler change that gets a <1% improvement on code. For domains where people are trying to squeeze out all the perf they can (i.e. the same people asking for ref-returns in the first place), such improves are huge. Remember that ref-returns are for people who still want to code in a safe language like c#, but want to be 'as close to the metal' as possible. i.e. things like game-engines and whatnot. In these domains these percentages matter. You may get a few more fps from this. Or this may open headroom for more features. We did ref-return in the first place precisely for perf. So getting more perf seems like a super great thing to be able to provide :) |
Right. My post was in the context of this proposal, where @controlflow made it sound as if 7% can indeed be a lot. But considering all the disadvantages of |
I think everybody basically agrees and is just having different reactions to some apparent trolling on @controlflow's part 😆 |
The thing that bothers me is that the examples don't really demonstrate the usefulness of this feature or the scenarios where this is useful. Now, I don't have issues with poor examples as long as I can understand what they try to convey. |
IU went back and looked at #118. Lo and behold, @controlflow downvoted the original ref return/local proposal. So they clearly are trolling here sadly and I, along with other, took the bait. 😠 |
In the current "let's sell out the language to the highest bigger" world of the C# language team, you are free to take that view if you want. And with your "7% sounds absolutely massive to me" attitude, the language team will no doubt help to lower the quality of the language further by encouraging premature/micro optimisation behaviour, such as the inappropriate use of In my world though (ironically stemming from some excellent teaching from game developers), mutation of values is always seen as a negative, except in edge cases. Clearly game engines are one such edge case and ref locals/returns are a huge quality win here as it removes the need for globally accessible state to avoid slow value copying. They are edge cases though and, as a norm, using |
Yes. |
Then don't mutate variables.
This feature is primarily for that edge case. 'ref' is extremely advanced, and is meant for those who need to squeeze out this sort of performance. We're not going to create the feature to address the problem for this set of users, and then go and hobble it so they can't use it. |
The language team will do what we always do. Assess the needs of customers and determine if and what we feel will be best for the language. If you want to change that, you're welcome to come join us and bring your own judgement to bear in that process. Until then, i imagine you'll have to continually be disappointed that we we don't have a special process for ensuring that the changes we make will meet with your approval before we do them :-/ |
I didn't explain that properly then as that is not what I was suggesting. The sort of advice I'd hope to see coming out of the language team would be:
What I'm hearing instead though, eg in response to the blatant misuse proposed here is: |
I literally just said: "This feature is primarily for that edge case. 'ref' is extremely advanced, and is meant for those who need to squeeze out this sort of performance."
I can't help you with that. When we're presenting this feature to new audiences we spend time discussing when and where this feature is appropriate and when and where it isn't. People presenting the information have routinely mentioned that it's for extremely niche scenarios, and unless you're someone working with arrays of large structs (like a game developer), and you need to operate on that data with as little overhead as possible, then this feature likely isn't relevant to you. I'm virtually certain i've seen presentations that said something akin to "in fact, this is likely so not-relevant to most people here, that we're not even going to spend further time on this, as there are vastly more relevant features that we should cover instead. If you're in a domain where you need to squeeze this sort of perf out of the system, you've probably already been wanting this, and can learn more. Otherwise, it's almost certainly not for you." But, given that, that doesn't mean that every sentence out of my mouth is going to repeat that information. At some point repeating the same information gets old, and it interferes with having an actual discussion about the topic. I get that you want the language team to design the language in a certain way. And message the language in a certain way. And do all sorts of things a certain way. But you have to accept that you're dealing with humans, operating with a whole host of challenges, and communicating with many different mediums. It's virtually guaranteed that across all that, it will be common that what we do and say doesn't align with what you want or what you want to hear. This is especially true in Github. I'm going to talk and discuss and weigh out issues. And trying to craft the message such that you find it acceptable, is simply not something that i'm going to spend brain cycles on.
Agreed. |
@DavidArno I can no longer make sense of your argument. I originally responded because you stated:
I disagree with this. And it was a core design goal of the feature that this be possible. Many (most?) of the customers that wanted this feature, also very much wanted mutation as well. Now you're saying that we should be messaging:
On one hand, you say that mutation of refs is a 'misuse'. Now you say that it can be appropriate to "write faster - and cleaner - code." and that for people with mutation needs "ref returns/locals are for you". How can it be a misuse when you yourself have identified precisely an audience for which this is an extremely desirable property of the system? To me, it sounds like you're saying exactly what was being said back to you. Namely that there are good reasons for this, even though you emphatically claimed "They aren't intended to allow you to start modifying such data." |
@DavidArno who do you think this 'highest bidder' was? In what way do you think the language has "sold out"? What does it even mean in the context of this discussion, or in the development of C#? Did we ever claim some cause that we would be somehow loyal to? Did we make claims about things we would/wouldn't do in the language? I've been working on this language for a large portion of my life. And the only driving desire i've ever had (and i've ever seen from the team), is to make a useful language for a broad spectrum of developers working on an enormously diverse set of tasks. Indeed, if it hasn't been clear by version 7, C# has been happy to adopt and adapt to all sorts of patterns and practices across the developer ecosystem. We don't claim any sort of ideological purity of our language. We're not pure OO, but we have aspects of OO systems in us. We're not functional, but we have aspects of functional languages in us. We're not dynamic, but we have embraced some dynamic features. We're not focused on low level programming, but we take in features to make that easier at times. We're not primarily concerned with any particular platform or domain. Want to use us for mobile? Great. For the server? Great. On the desktop? Great. On a TV? Go for it. Want to use us on MS' stack, please do. On someone else's stack? Be our guest. Use us far and wide. etc. etc. We want our language to be useful to an incredibly broad spectrum of developers. Part of that spectrum includes people who are trying to squeeze every last drop of performance in heavily constrained scenarios. With the help of them, and others, we identified a very cheap and simple feature change to the language that helped them out enormously. These customers got huge wins, with just a small amount of work from us (that we think will complement future changes we want to do in the language). In this case, we found that we could provide high bang, for low buck, for a small audience. In balance, we felt that this was worth it. If you feel like that's "selling out", then i'm not sure what to tell you. Except that we will continue to do this sort of thing as we're constantly seeking out ways to make our language better (sometimes for the majority, and sometimes for a minority). |
I was making a generalisation. Clearly I shouldn't have done. My precise position on them, rather than my previous generalisation, would be:
I want to express that as "ref returns aren't intended to allow you to start modifying such data. Their purpose is to allow optimised returns that expose lots of value data without having to duplicate that data when returning. Using ref returns to modify that data is a misuse of this feature." and know that those that need to modify that data will ignore this generalised advice; but I'll retract that and go for my less absolute version instead. |
I think that people were too hooked by the examples above that at first I didn't even understand what's the point of the proposal. When you read something like this:
It makes you think whether it's actually a joke because it's written as if setters are obsolete and we should mutate variables directly now. Just my 2c. |
This is below you. Baseless and inappropriate. Disappointed. |
You are probably right. I like to write that way, but I probably need to tone it down a bit at times (or maybe tone it down a lot, a lot of the time). |
Then what is the point of |
As i've already explained, i do not view it as undesirable. I view it as a core part of what the feature is. And, as already mentioned several times: i want us to have "readonly refs" and i hope we will do that in the future.
It would make sense to say: if you don't want to do something that C# allows, and C# provides no way to stop you from doing it, then don't do it. C# isn't going to add features to make every single thing that David doesn't like disallowed. :) |
And that right there gets to the heart of the issue. Fix this and C# will be the most most perfect language, ever (until I change my mind next week on what ought to be disallowed of course) 😆 |
Closing this out. We're doing all language design now at dotnet/csharplang. If you're still interested in this idea let us know and we can migrate this over to a discussion in that repo. Thanks! |
So, currently in C# 7.0 we have old boring auto-properties with set accessors:
And a new efficient ref-returning get-only (we all hate setters!) modern property pattern:
New modern property pattern has improved performance of compound operators:
And more importantly: ability to pass modern properties as an argument to
ref
parameter (we all waited this for years!!):With all that benefits in mind, I don't see a single reason to use old-school properties again! The only problem is the verbose syntax of modern property pattern, requiring backing field declaration. Using
ref
modifier with get-only auto-property syntax produces error for some reason:Dear Roslyn team, please allow get-only auto-properties to return by reference to allow concise way to express modern property pattern.
The text was updated successfully, but these errors were encountered: