Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove scope variance exceptions #76296

Merged
merged 7 commits into from
Jan 6, 2025

Conversation

jjonescz
Copy link
Member

@jjonescz jjonescz commented Dec 6, 2024

Fixes #76100.
Alternative to #76261.
Speclet update: dotnet/csharplang#8777

@dotnet-issue-labeler dotnet-issue-labeler bot added Area-Compilers untriaged Issues and PRs which have not yet been triaged by a lead labels Dec 6, 2024
@jjonescz jjonescz force-pushed the 76100-ScopeVariance-NoExceptions branch from 493ac3a to f0d3f46 Compare December 6, 2024 09:50
@jjonescz jjonescz marked this pull request as ready for review December 6, 2024 11:51
@jjonescz jjonescz requested a review from a team as a code owner December 6, 2024 11:51
@jjonescz jjonescz requested review from jaredpar and cston December 9, 2024 10:51
@@ -118,3 +118,22 @@ class C
}
```

## Variance of `scoped` and `[UnscopedRef]` is more strict
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I found these breaks when building the BCL with this change: dotnet/runtime@fae33d2e25a

Copy link
Member Author

Choose a reason for hiding this comment

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

@jaredpar jaredpar removed the untriaged Issues and PRs which have not yet been triaged by a lead label Dec 10, 2024
@jjonescz jjonescz requested a review from jaredpar December 10, 2024 13:39
Copy link
Member

@333fred 333fred left a comment

Choose a reason for hiding this comment

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

I much prefer the consistency here, thanks!

// We have removed exceptions to the scoped mismatch error reporting, but to avoid breaks
// we report the new scenarios (previously exempted) as warnings in C# 12 and earlier.
// https://github.com/dotnet/roslyn/issues/76100
(overrideMethod.DeclaringCompilation.LanguageVersion > LanguageVersion.CSharp12 || usedToBeReported(baseMethod));
Copy link
Member Author

Choose a reason for hiding this comment

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

Downgraded the new errors to warnings in C# 12 and lower to lessen the break per offline discussion with Jared. (We think breaking in C# 13 is fine since it's new.)

@jjonescz jjonescz requested a review from 333fred December 12, 2024 21:07
@jjonescz jjonescz enabled auto-merge (squash) December 13, 2024 09:51
@jjonescz jjonescz disabled auto-merge December 13, 2024 11:37
@jjonescz
Copy link
Member Author

@cston @dotnet/roslyn-compiler for a second review, thanks

@jjonescz jjonescz enabled auto-merge (squash) January 6, 2025 09:18
@jjonescz jjonescz merged commit b86d831 into dotnet:main Jan 6, 2025
24 checks passed
# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

UnscopedRef can be different in interface implementation, leading to ref safety holes
4 participants