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

DefaultInterfaceImplementation open issues and work items #17952

Closed
27 of 42 tasks
AlekseyTs opened this issue Mar 18, 2017 · 2 comments
Closed
27 of 42 tasks

DefaultInterfaceImplementation open issues and work items #17952

AlekseyTs opened this issue Mar 18, 2017 · 2 comments
Assignees
Labels
Area-Compilers Feature - Default Interface Impl Default Interface Implementation Feature Request Test Test failures in roslyn-CI
Milestone

Comments

@AlekseyTs
Copy link
Contributor

AlekseyTs commented Mar 18, 2017

Open issues:

  • There seems to be a confusion about whether scenarios from tests MethodImplementation_021, MethodImplementation_031 and MethodImplementation_041 in src/Compilers/CSharp/Test/Symbol/Symbols/DefaultInterfaceImplementationTests.cs were discussed at the LDM and, if that discussion happened, what was the decision about the expected behavior. I believe that the decision was made to report errors similar to those that would be reported if default implementation wasn't provided. However, it looks like that part of discussion didn't make it to the notes. Based on the feedback provided by @gafter during code review, the implementation was adjusted to not report any errors in those scenarios. Need to bring these scenarios to LDM again and clarify what the expected behavior should be.
  • None of the tests are running PEVerify or verify expected behavior by running the compiled code because, at the moment, we cannot target runtime that supports the feature. The tests should be adjusted once they are able to target a runtime with required support.
  • The feature is currently targeting 7.1 language version, that might not be the final plan.
  • Are there any implications around _VtblGap methods?
  • Should we make error message for ERR_InterfaceMemberNotFound (reported when an implementable member matching an explicit interface implementation is not found) provide more details (member is present, but not virtual, etc.)?
  • We currently (Add initial support for specifying explicit base type in the base access. #33281) generate explicit interface implementations with the same access as the method that is implemented, but that may not permit an explicit interface implementation to be accessible (from the CLR's point of view) to a base invocation that has access to it. We might use protected access to make it work, but that requires that we define the CLR semantics of protected members in interfaces. See also Issues in Default Interface Methods csharplang#406.

Work items:

  • Disable embedding of interfaces with default implementations (NoPia).
  • Deal with base references within the default implementation.
  • Deal with interaction with dynamic invocations for base invocations.
  • Deal with interaction with dynamic and runtime support.
  • Test WinMD events.
  • Test unsafe modifiers on members with implementation and fields (anything interesting there)?
  • Cover "private protected" when it goes live.
  • Need to import required modifiers in all signatures, if there is default implementation of such member, we should be able to detect that and then the program being compiled is off the hook.
  • It should not be possible to override/implement inaccessible interface members. From LDM-2017-04-18:

DIM implementing inaccessible interface members (not in list)

The way the runtime works today, a class member can happily implement an interface member that isn't accessible! That's not likely to be dependent on today (no language will generate that interface), but we need to decide what semantics to have here.

We could continue to only have public members in interfaces be virtual. But if we want protected, internal and private, we should probably have it so they can only be implemented by classes that can see them. But this means that interfaces can prevent other assemblies from implementing them! This may be a nice feature - it allows closed sets of implementations.

Conclusion

This is still open, but our current stake in the ground is we should allow non-public virtual interface members, but disallow overriding or implementing them in places where they are not accessible.

  • Disallow implicit implementation of non-public interface members. From LDM-2017-04-18:

DIM Implementing a non-public interface member (not in list)

Would we allow them to be implemented implicitly? If so, what is required of the accessibility of the implementing method?:

  • Must be public
  • Must be the same accessibility
  • Must be at least as accessible

Conclusion

For now, let's not allow any of these. We can relax as we think through it.

Language features not yet implemented

  • Support abstract modifier in interfaces
  • Support public modifier in interfaces
  • Support virtual modifier in interfaces
  • Support override declarations in interfaces, using explicit implementation syntax
  • Support override declarations in interfaces, using implicit selection of overridden method(s)
  • Support for properties and indexers
    • Partially abstract (only one of get/set has a body)
  • Support "reabstraction": an abstract override declaration in an interface. Note: We are using an explicit implementation syntax which doesn't support reabstraction, so a syntax change in needed.
  • Detect and report "most specific override" rule violation in classes and interfaces
  • Detect missing implementation of a reabstracted method in an abstract class
  • Support private instance members in interfaces
  • Support private static members in interfaces
  • Support public static members in interfaces
  • Support interface base qualifier base(Type).M() in interfaces and classes Default Interface Method Base Calls #32054
  • Declaring protected and protected internal types within interfaces.
  • Detecting and reporting cycles in interface base clauses.

Other non-compiler work

  • Coordinate release timing with various runtime platforms
  • Develop a plan for interoperation with VB, managed C++, and F#
  • Make sure ENC (adding/editing members) works or fails gracefully.
  • ENC: Adding a new instance member (even without a body) is not reported as a rude edit, but breaks the debugging session (changes aren't picked up properly).
  • Completion for access modifiers and probably other modifiers doesn't work.
  • After typing an interface name in an explicit interface implementation, completion suggests to implement members that aren't implementable (non-virtual instance, or static methods).
  • The fix to implement an interface (explicitly or implicitly) spills implementation for not-implementable members (static/sealed).
  • No recommendation/completion for types within base(...) expression.
  • For base access, when the explicit base is an interface, interface members are not suggested, members of the immediate base class are suggested instead.
  • For base access, when the explicit base is a class, members of the immediate base class are suggested regardless of the actual class specified.
@jcouv
Copy link
Member

jcouv commented Mar 29, 2018

Some additional test ideas:

  • Test with SkipLocalsInitialization attribute

@NinoFloris
Copy link

This may be a relevant scenario to work through as well
#35008

@jinujoseph jinujoseph modified the milestones: 16.5, 16.6 Mar 22, 2020
@jaredpar jaredpar modified the milestones: 16.6, Backlog Apr 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Feature - Default Interface Impl Default Interface Implementation Feature Request Test Test failures in roslyn-CI
Projects
None yet
Development

No branches or pull requests

7 participants
@jaredpar @agocke @NinoFloris @AlekseyTs @jinujoseph @jcouv and others