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.

Sign up for GitHub

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

Consider silly cyclic assignment in scoped variance #8766

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Consider silly cyclic assignment in scoped variance #8766

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 5 additions & 2 deletions proposals/csharp-11.0/low-level-struct-improvements.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -469,8 +469,11 @@ The `scoped` modifier and `[UnscopedRef]` attribute (see [below](#rules-unscoped
Any other difference with respect to `scoped` or `[UnscopedRef]` is considered a mismatch.

The compiler will report a diagnostic for _unsafe scoped mismatches_ across overrides, interface implementations, and delegate conversions when:
- The method returns a `ref struct` or returns a `ref` or `ref readonly`, or the method has a `ref` or `out` parameter of `ref struct` type, and
- The method has at least one additional `ref`, `in`, or `out` parameter, or a parameter of `ref struct` type.
- The method has a `ref` or `out` parameter of `ref struct` type with a mismatch of adding `[UnscopedRef]` (not removing `scoped`).
(In this case, a [silly cyclic assignment](#cyclic-assignment) is possible, hence no other parameters are necessary.)
Copy link
Member

Choose a reason for hiding this comment

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

I don't think it's so much that it allows silly cyclic assignment as it's just a basic type system violation. Consider that it allows you to do things like:

interface I1 {
  ref int M(); 
}
struct S {
  int field;
  [UnscopedRef] ref int M() => ref field;
}

This is a similar case where introducing [UnscopedRef] in the implementing type leads to lifetime violation.

Copy link
Member Author

@jjonescz jjonescz Dec 4, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes, but that example is already covered by the existing rules and is an error today (I fixed it so S actually implements I1). Those rules are

  • Or both of these are true:
    • The method returns a ref struct or returns a ref or ref readonly, or the method has a ref or out parameter of ref struct type.
    • The method has at least one additional ref, in, or out parameter, or a parameter of ref struct type.

This new bullet point

  • The method has a ref or out parameter of ref struct type with a mismatch of adding [UnscopedRef] (not removing scoped).

is only necessary because of the silly cyclic assignment.

Copy link
Member Author

Choose a reason for hiding this comment

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

Discussed offline and it's not clear why the paragraph starting with

The compiler will report a diagnostic for unsafe scoped mismatches across overrides, interface implementations, and delegate conversions when:

is in the speclet in the first place.

In other words, it seems that adding [UnscopedRef] or removing scoped should always be a mismatch error without exceptions. cc @cston

We should also investigate this sentence in the light of ref structs being allowed to implement interfaces:

The rules above ignore this parameters because ref struct instance methods cannot be used for overrides, interface implementations, or delegate conversions.

- Or both of these are true:
- The method returns a `ref struct` or returns a `ref` or `ref readonly`, or the method has a `ref` or `out` parameter of `ref struct` type.
- The method has at least one additional `ref`, `in`, or `out` parameter, or a parameter of `ref struct` type.

The rules above ignore `this` parameters because `ref struct` instance methods cannot be used for overrides, interface implementations, or delegate conversions.

Expand Down