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

C# compiler should issue CS1961 for variance violations for local functions within default interface methods #39731

Closed
colinjneville opened this issue Nov 8, 2019 · 3 comments · Fixed by #39839
Assignees
Labels
Area-Compilers Bug Feature - Default Interface Impl Default Interface Implementation Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Milestone

Comments

@colinjneville
Copy link

Version Used:
Visual Studio 16.3.8
.Net Core 3.0
csc 3.3.1-beta3-19461-02

Steps to Reproduce:

Example repro (either interface will do)

public interface ICovariant<out T> {
    void DefaultMethod() {
        static void LocalFunction(T t) { }
    }
}

public interface IContravariant<in T>{
    void DefaultMethod() {
        static T LocalFunction() => default;
    }
}

class Program {
    static void Main(string[] args) {
        ICovariant<object> covariant;
        IContravariant<object> contravariant;
    }
}

Expected Behavior:
A local function which uses a variant generic type parameter incorrectly should cause a CS1961 'invalid variance' error if the local function is contained within a default interface method.

Actual Behavior:

The program will compile successfully, but throw at runtime when loading the containing type:

System.TypeLoadException
  HResult=0x80131522
  Message=Could not load type 'ICovariant`1' from assembly 'ScratchCore, Version=1.0.0.0, Culture=neutral, PublicKeyToken=null' because a covariant or contravariant type parameter was used illegally in the signature for an argument in method '<DefaultMethod>g__LocalFunction|0_0'.
  Source=<Cannot evaluate the exception source>
  StackTrace:
<Cannot evaluate the exception stack trace>
@gafter gafter added this to the Compiler.Next milestone Nov 11, 2019
@gafter
Copy link
Member

gafter commented Nov 11, 2019

Agreed. Also, the specification should include appropriate constraints that require the diagnostic. Assigning to @AlekseyTs for the compiler work and @gafter for the specification work.

@AlekseyTs
Copy link
Contributor

It feels like an existing section 13.1.3.1 Variance safety in C# Language Specification already covers this.

@AlekseyTs
Copy link
Contributor

After an offline conversation with @gafter we arrived to the following:

  • There are no variance safety issues with local functions in the scenarios described above. Therefore, no errors should be reported.
  • The program should run successfully. In order to achieve that nested classes synthesized by the compiler should not have variance in their type parameters.
  • Declaration of classes, structures and enums within variant interfaces should be disallowed (this limitation should be reflected in the specification). VB compiler already does this.

AlekseyTs added a commit to AlekseyTs/roslyn that referenced this issue Nov 15, 2019
@gafter gafter removed their assignment Nov 19, 2019
AlekseyTs added a commit that referenced this issue Nov 20, 2019
…39839)

Fixes #39731.

- Declaration of classes, structures and enums within variant interfaces is disallowed.
- Nested classes synthesized by the compiler don't have variant emitted type parameters.
- Local functions and lambdas used within a variant interface always have display class.
@AlekseyTs AlekseyTs added the Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented label Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Feature - Default Interface Impl Default Interface Implementation Resolution-Fixed The bug has been fixed and/or the requested behavior has been implemented
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants