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

Handle ref safety analysis for type parameters with 'allows ref struct' constraint #73281

Conversation

AlekseyTs
Copy link
Contributor

No description provided.

@AlekseyTs AlekseyTs requested a review from a team as a code owner April 30, 2024 13:41
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Issues and PRs which have not yet been triaged by a lead label Apr 30, 2024
@AlekseyTs AlekseyTs requested review from cston and jjonescz April 30, 2024 13:41
@AlekseyTs
Copy link
Contributor Author

@cston, @jjonescz, @dotnet/roslyn-compiler Please review

@AlekseyTs
Copy link
Contributor Author

@cston, @jjonescz, @dotnet/roslyn-compiler Please review

1 similar comment
@AlekseyTs
Copy link
Contributor Author

@cston, @jjonescz, @dotnet/roslyn-compiler Please review

case BoundKind.NewT:
{
var newT = (BoundNewT)expr;
var constructorSymbol = newT.Constructor;
Copy link
Member

@cston cston May 1, 2024

Choose a reason for hiding this comment

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

constructorSymbol

Not used. #Resolved

@@ -663,7 +663,9 @@ private BoundExpression EvaluateSideEffects(BoundExpression arg, RefKind refKind

#if DEBUG
var discardedUseSiteInfo = CompoundUseSiteInfo<AssemblySymbol>.Discarded;
Debug.Assert(_compilation.Conversions.ClassifyConversionFromType(rewrittenReceiver.Type, memberSymbol.ContainingType, isChecked: false, ref discardedUseSiteInfo).IsImplicit);
// PROTOTYPE(RefStructInterfaces): Test various flavors of object/collection initializers on a type parameter that allows ref struct
Debug.Assert(_compilation.Conversions.ClassifyConversionFromType(rewrittenReceiver.Type, memberSymbol.ContainingType, isChecked: false, ref discardedUseSiteInfo).IsImplicit ||
Copy link
Member

@cston cston May 1, 2024

Choose a reason for hiding this comment

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

_compilation.Conversions.ClassifyConversionFromType(rewrittenReceiver.Type, memberSymbol.ContainingType, isChecked: false, ref discardedUseSiteInfo).IsImplicit ||

It looks like this check can be dropped since it is included in HasImplicitConversionToOrImplementsVarianceCompatibleInterface(). #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It looks like this check can be dropped since it is included in HasImplicitConversionToOrImplementsVarianceCompatibleInterface().

It actually isn't. HasImplicitConversionToOrImplementsVarianceCompatibleInterface deals only with interfaces.


RS M1()
{
// RSTE of `this` is CurrentMethod
Copy link
Member

@cston cston May 1, 2024

Choose a reason for hiding this comment

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

this

this is an interface, not a type parameter with allows ref struct. #Resolved

Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is an interface, not a type parameter with allows ref struct.

And the point you are trying to make is?

Copy link
Member

Choose a reason for hiding this comment

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

The test didn't seem particularly interesting to me because this is an interface and not by-ref-like. But the behavior (no errors reported) looks correct.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The behavior is different by comparison to the cloned test, but correct. That feels good enough to keep the test.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature - RefStructInterfaces untriaged Issues and PRs which have not yet been triaged by a lead
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants