Specially handle more scenarios for type parameters with 'allows ref struct' constraint #73059
Conversation
…struct' constraint
dotnet-issue-labeler
bot
added
the
untriaged
|
|
||
| [Theory] | ||
| [CombinatorialData] | ||
| public void AnonymousDelegateType_01_ActionDisallowsRefStruct(bool s2IsRefStruct) |
There was a problem hiding this comment.
Consider testing a scenario where we infer delegate type from a method with a parameter that has a default value (so we are forced to synthesize a delegate type for that) - the resulting synthesized delegate type should have T : allows ref struct even if it's not used with a ref struct or "allows ref struct" type parameter. (And it should not have that constraint on Net7.) #Resolved
There was a problem hiding this comment.
Consider testing a scenario where we infer delegate type from a method with a parameter that has a default value (so we are forced to synthesize a delegate type for that) - the resulting synthesized delegate type should have
T : allows ref structeven if it's not used with a ref struct or "allows ref struct" type parameter. (And it should not have that constraint on Net7.)
I do not find this scenario interesting. Constraints on type parameters do not depend on specific type arguments used for specific call sites. Effects of default values and specific types involved into specific signature are not related to each other. In my opinion the added tests adequately cover affected code paths and various flavors of generic anonymous delegate types. Please let me know if you disagree.
There was a problem hiding this comment.
BTW, there is a test with a pointer type, which is not a ref struct.
|
@cston, @dotnet/roslyn-compiler For the second review. |
2 similar comments
|
@cston, @dotnet/roslyn-compiler For the second review. |
|
@cston, @dotnet/roslyn-compiler For the second review. |
| get { return false; } | ||
| get | ||
| { | ||
| return _container.IsDelegateType() && _container.Manager.Compilation.SourceAssembly.RuntimeSupportsByRefLikeGenerics; |
There was a problem hiding this comment.
Manager.Compilation.SourceAssembly
Consider simplifying to
ContainingAssembly.
Will do for the next PR.
The original test (before Refers to: src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs:17484 in 26c51ef. [](commit_id = 26c51ef, deletion_comment = False) |
| var comp = CreateCompilation(src, targetFramework: s_targetFrameworkSupportingByRefLikeGenerics, options: TestOptions.ReleaseExe); | ||
|
|
||
| var verifier = CompileAndVerify( | ||
| comp, expectedOutput: ExecutionConditionUtil.IsMonoOrCoreClr ? @"<>F`2[S1,S2] Test1 S1" : null, |
| namespace System | ||
| { | ||
| public delegate void Action<T1, T2>(T1 x, T2 y); | ||
| } |
There was a problem hiding this comment.
Do we need both this definition and TargetFramework.Net70? Same question for the next test. #Resolved
There was a problem hiding this comment.
Do we need both this definition and TargetFramework.Net70? Same question for the next test.
I prefer to explicitly set up the desired test environment.
Will do in the next PR In reply to: 2066991719 Refers to: src/Compilers/CSharp/Test/Emit3/RefStructInterfacesTests.cs:17484 in 26c51ef. [](commit_id = 26c51ef, deletion_comment = False) |
AlekseyTs
merged commit 1841565
into
dotnet:features/RefStructInterfaces
No description provided.