Disallow Clone members in source records#45775
Conversation
| ERR_DoesNotOverrideMethodFromObject = 8869, | ||
| ERR_SealedGetHashCodeInRecord = 8870, | ||
| ERR_DoesNotOverrideBaseEquals = 8871, | ||
| ERR_CloneDisallowedInRecord = 8872, |
There was a problem hiding this comment.
8872 [](start = 38, length = 4)
There is already a PR under review that uses numbers through 8874. Consider picking one of the next numbers and also leaving an empty line above this member to simplify merging process. #Closed
| return; | ||
| } | ||
|
|
||
| Dictionary<string, ImmutableArray<Symbol>> membersByName = GetMembersByName(); |
There was a problem hiding this comment.
GetMembersByName [](start = 71, length = 16)
Could we simply use GetMembers("Clone") and then iterate through the result? #Closed
|
Done with review pass (iteration 1) #Closed |
Co-authored-by: Rikki Gibson <rikkigibson@gmail.com>
| var comp = CreateCompilationWithIL(src, il, options: TestOptions.DebugExe, parseOptions: TestOptions.RegularPreview); | ||
| comp.VerifyDiagnostics(); | ||
| CompileAndVerify(comp, expectedOutput: "RAN"); | ||
| // Note: we do load the Clone method from metadata |
There was a problem hiding this comment.
That is an interesting case. What is the rationale for allowing this vs. marking this as an error in metadata import?
I don't necessarily think this is wrong but curious about the reasoning here.
There was a problem hiding this comment.
@jaredpar I'm open to revisiting. My rationale was:
(1) we're putting a restriction with the idea of lifting it in the future, it may not be worthwhile to be airtight to catch people who really go out of their way to do weird things.
(2) not knowing what the future design would be and whether it would require a new framework, I'm not sure we want to prevent the C# 9 compiler from using types produced by a future compiler (ie. that do have speakable Clone methods).
|
@dotnet/roslyn-compiler for a second review. Thanks |
|
Was hoping to redeclare as |
|
@alrz In the current design, |
Fixes #45591
Corresponding spec change: dotnet/csharplang#3652