Disallow pointer and restricted types for record parameters#48925
Disallow pointer and restricted types for record parameters#48925jcouv merged 7 commits intodotnet:masterfrom
Conversation
77fad72 to
d6447ea
Compare
d6447ea to
be9a062
Compare
| foreach (var parameter in ctor.Parameters) | ||
| { | ||
| var parameterType = parameter.Type; | ||
| if (parameterType.IsPointerOrFunctionPointer() || parameterType.IsRestrictedType()) |
There was a problem hiding this comment.
IsPointerOrFunctionPointer [](start = 42, length = 26)
Should we use IsUnsafe helper instead? It looks like this is what GetAnonymousTypeFieldType is using and it looks like IsUnsafe flags more types. #Closed
There was a problem hiding this comment.
The question is should we align with type argument rules or anonymous type rules.
I included tests with int*[] to highlight the difference.
Raised question in thread with LDM
In reply to: 512784802 [](ancestors = 512784802)
There was a problem hiding this comment.
Thread from LDM landed on disallowing unsafe types. Pushed the change to the PR. Thanks
In reply to: 513087857 [](ancestors = 513087857,512784802)
| var existingOrAddedMembers = addProperties(ctor.Parameters); | ||
| addDeconstruct(ctor, existingOrAddedMembers); | ||
|
|
||
| foreach (var parameter in ctor.Parameters) |
There was a problem hiding this comment.
foreach (var parameter in ctor.Parameters) [](start = 20, length = 42)
I am not sure this is the right place for the check and the right set of members to check. If I remember correctly, any field in the type is a subject for equality. #Closed
There was a problem hiding this comment.
| static class C2 { } | ||
| ref struct RefLike{} | ||
|
|
||
| unsafe record C( // 1 |
There was a problem hiding this comment.
Please add similar test(s) for fields and auto-properties explicitly declared in a record. #Closed
|
Done with review pass (iteration 1) #Closed |
|
@dotnet/roslyn-compiler for review. Thanks |
| var parameterType = f.Type; | ||
| if (parameterType.IsUnsafe()) | ||
| { | ||
| diagnostics.Add(ErrorCode.ERR_BadFieldTypeInRecord, f.Locations[0], parameterType); |
There was a problem hiding this comment.
[0] [](start = 91, length = 3)
Since we do not control what kind of symbols we are going to get here, consider using FirstOrNone helper here.
|
|
||
| [ConditionalFact(typeof(DesktopOnly), Reason = ConditionalSkipReason.RestrictedTypesNeedDesktop)] | ||
| [WorkItem(48115, "https://github.com/dotnet/roslyn/issues/48115")] | ||
| public void RestrictedTypesAndPointerTypes_NominalMembers() |
There was a problem hiding this comment.
RestrictedTypesAndPointerTypes_NominalMembers [](start = 20, length = 45)
Please add similar test(s) for auto-properties explicitly declared in a record.
AlekseyTs
left a comment
There was a problem hiding this comment.
LGTM (iteration 5), modulo a test suggestion and suggestion to use FirstOrNone helper for location.
Fixes #48115
We're disallowing unsafe types and restricted types for record fields.
Spec update: dotnet/csharplang#4077