-
Notifications
You must be signed in to change notification settings - Fork 19
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
chore: Add common shims for new language features #182
Conversation
a0e63dc
to
da00f1c
Compare
Handful of commonly used shim types to unlock new(-ish) language features in older TFMs. Limited to features that are only gated by symbol existence checks, but covers some fairly common features. Signed-off-by: Austin Drenski <[email protected]>
da00f1c
to
429d87a
Compare
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
Signed-off-by: André Silva <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some of this is a bit beyond me, though the reasoning from @austindrenski makes sense.
@askpt I won't block this, but I don't feel confident enough to approve it.
@kinyoklion @benjiro how do you feel about this? Do you see risks here?
I think that actually following the example provided backward through its journey would express the type of concern I have. Is a fix for a previously incorrectly shimmed item, which was originally reported on the dotnet runtime. So, by shimming that feature it created a multi-party chain of issues and corrections. Which maybe, based on guidance from the issues, we will not encounter. I think you get some ergonomic improvements in exchange for unexpected and confusing experiences when things go wrong. I do think that the pattern in Thank you, |
@toddbaert and @kinyoklion: I will close this PR and add new shims as needed. I was not the original proponent for this PR, so I don't feel comfortable merging it if the team is also uncomfortable. I am just trying to unblock all the breaking changes at the moment. |
Thanks @askpt, much appreciated. 🙏 |
Handful of commonly used shim types to un-gate new(-ish) language features in older TFMs. Limited to features that are only gated by symbol existence checks, but covers some fairly common features.
See: #264, #181
tldr; I kept reaching for these features while drafting #181, so figured I'd open a standalone PR to see if there are any blocking concerns so these don't just sneak in as part of #181.
If accepted here, will copy/pasta to the contrib repo as well.
FAQ
This seems sketchy? Is it safe?
It's not not sketchy, but you can find similar examples of these types of un-feature-gating shims around the .NET ecosystem (e.g. StackExchange/StackExchange.Redis#2621).
The key to playing with these bits is making sure you don't start reimplementing logic vs placing type definitions in scope for Roslyn to consume. That way these types disappear at compile-time and Roslyn's normal bits take their place.
What's the argument in favor of this? Is there added complexity?
Given that this project will likely want to maintain compat for
netstandard2.0
andnet462
until they EOL (far into the future), many new language features will remain gated just out-of-reach, so when/where possible, low-hanging shims can bring some fun to the otherwise dreary world ofnet462
and friends.Those features are often times just sugar, but sometimes that sugar is useful (e.g. record syntax), and other times it's just more familiar and can lower the barrier to new contributors.