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

Consider silly cyclic assignment in scoped variance #8766

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

Conversation

jjonescz
Copy link
Member

@jjonescz jjonescz commented Dec 4, 2024

No description provided.

- 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

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants