Support static constructors in interfaces #33625
Conversation
AlekseyTs
added
Area-Compilers
Feature - Default Interface Impl
|
@dotnet/roslyn-compiler Please review |
2 similar comments
|
@dotnet/roslyn-compiler Please review |
|
@dotnet/roslyn-compiler Please review |
|
Reviewing |
|
|
||
| compilation1.VerifyDiagnostics(); | ||
|
|
||
| CompileAndVerify(compilation1, expectedOutput: !CoreClrShim.IsRunningOnCoreClr ? null : "I1.I1", verify: VerifyOnCoreClr); |
There was a problem hiding this comment.
The runtime has facilities for dealing with cyclic static constructors on classes / structs. The behavior should be the same for interfaces (largely undefined ordering but executes without crashing). Think we should add one sanity test here just to ensure they have hooked up this code path.
There was a problem hiding this comment.
The runtime has facilities for dealing with cyclic static constructors on classes / structs. The behavior should be the same for interfaces (largely undefined ordering but executes without crashing). Think we should add one sanity test here just to ensure they have hooked up this code path.
Sure, let's discuss offline what you have in mind in more details.
In reply to: 260422959 [](ancestors = 260422959)
| parseOptions: TestOptions.Regular7_3); | ||
|
|
||
| compilation1.VerifyDiagnostics( | ||
| // (4,12): error CS8652: The feature 'default interface implementation' is currently in Preview and *unsupported*. To use Preview features, use the 'preview' language version. |
There was a problem hiding this comment.
Nit: is there any way to suppress this error? Otherwise, when we eventually ship C# 8.0, this error will need to be deleted from this test and who knows how many others. #Resolved
There was a problem hiding this comment.
Nit: is there any way to suppress this error? Otherwise, when we eventually ship C# 8.0, this error will need to be deleted from this test and who knows how many others.
It is the purpose of the test to verify presence of this diagnostics. The error will never go away for the scenario, the wording might change though, but that is due to the "Preview" mode ending.
In reply to: 260427140 [](ancestors = 260427140)
| // I1(){} | ||
| Diagnostic(ErrorCode.ERR_InterfacesCantContainConstructors, "I1").WithLocation(4, 5), | ||
| // (6,12): error CS0526: Interfaces cannot contain constructors | ||
| // (10,5): error CS0501: 'I2.I2()' must declare a body because it is not marked abstract, extern, or partial |
There was a problem hiding this comment.
Could we suppress this error if CS0526 is reported? This error doesn't matter: whether or not the interface constructor has a body is irrelevant. It's illegal either way. #Resolved
There was a problem hiding this comment.
|
|
||
| validate(compilation1.SourceModule); | ||
|
|
||
| void validate(ModuleSymbol m) |
There was a problem hiding this comment.
Minor nit: local method seems unnecessary. If you want to keep it, please move to the end of the function. #Pending
No description provided.