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

C# 7.x: Add support for readonly structs #333

Merged
merged 3 commits into from
Oct 7, 2022

Conversation

RexJaeschke
Copy link
Contributor

@RexJaeschke RexJaeschke commented Jun 15, 2021

This PR adds one new modifier to structs, readonly, in struct_modifier.

@RexJaeschke RexJaeschke added this to the C# 7.x milestone Jun 15, 2021
@KalleOlaviNiemitalo
Copy link
Contributor

Should an implicit conversion from dynamic to a ref struct type be rejected at compile time? Microsoft's implementation of dynamic uses the ref struct as a type argument, which is then not supported by the CLR. However, as the C# standard does not specify the details of dynamic, I imagine a different compiler with a different library could find a way to make the conversion work. dotnet/csharplang#4713, #282

@RexJaeschke RexJaeschke added the Review: pending Proposal is available for review label Sep 16, 2021
@gafter
Copy link
Member

gafter commented May 16, 2022

This draft does not reflect the draft specification at https://github.com/dotnet/csharplang/blob/main/proposals/csharp-7.2/span-safety.md

@RexJaeschke
Copy link
Contributor Author

Reminder to look at #334 for other open issues w.r.t this PR (including Neal's concerns re missing text re span safety).

@RexJaeschke
Copy link
Contributor Author

Based on Neal’s suggestion during yesterday's TG2 meeting, we agreed to merge as is, but only call readonly structs completed. Then, create another PR for all the additional work for ref structs.

@BillWagner
Copy link
Member

I'll rebase this branch to resolve conflicts, and mark it ready for review.

@BillWagner BillWagner force-pushed the Rex-v7-readonly-ref-struct branch from d0d7985 to 0178851 Compare June 9, 2022 14:08
@BillWagner BillWagner marked this pull request as ready for review June 9, 2022 14:08
@jskeet
Copy link
Contributor

jskeet commented Jul 13, 2022

Assigned to @gafter for the splitting off - we should have assigned this before...

@gafter gafter mentioned this pull request Jul 19, 2022
@gafter gafter changed the title C# 7.x: Add support for readonly and ref structs C# 7.x: Add support for readonly structs Jul 19, 2022
@gafter
Copy link
Member

gafter commented Jul 19, 2022

I've removed ref struct from this PR and moved it to #601. This should be fine for readonly struct.

@gafter gafter removed their assignment Jul 19, 2022
@BillWagner BillWagner force-pushed the Rex-v7-readonly-ref-struct branch from 044337b to 739d370 Compare October 2, 2022 22:18
@BillWagner
Copy link
Member

@jskeet

We should be able to do the final approval for this and merge it during this week's meeting. The ref struct portion has all been moved to #601

Copy link
Member

@BillWagner BillWagner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This LGTM. Let's :shipit:

@jskeet
Copy link
Contributor

jskeet commented Oct 7, 2022

Yup, just done a final review. Will merge now.

@jskeet jskeet merged commit 501b50d into draft-v7 Oct 7, 2022
@jskeet jskeet deleted the Rex-v7-readonly-ref-struct branch October 7, 2022 14:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review: pending Proposal is available for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants