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

Proposal: "Closed" type hierarchies #8729

Closed
agocke opened this issue Feb 16, 2016 · 16 comments
Closed

Proposal: "Closed" type hierarchies #8729

agocke opened this issue Feb 16, 2016 · 16 comments

Comments

@agocke
Copy link
Member

agocke commented Feb 16, 2016

Background

This proposal is substantially similar to #188, but it focuses more on integrating with existing C# object structures and less on adding new "ADT"-style objects. The goal, however, is still very similar. By adding closed type hierarchies the compiler can be guaranteed that all the types its sees are the only subclasses of the target type. This can integrate with features like pattern matching to provide a warning when not all possible types are checked in a pattern match.

Problem

Providing an ADT-like pattern is helpful for the pattern matching case but it unfortunately doesn't integrate well with current object-oriented patterns written in C#. For example, the Roslyn TypeSymbol type could be useful to be matched on, but the design as proposed in #188 would require rewriting or significantly modifying the existing type structure meet that goal. It would be better if, instead, the existing object hierarchy could be easily marked as closed, meaning that all inheriting classes must be in the same compilation unit as the definition.

Solution

The cleanest solution would simply be to add support for a new modifier, closed, that would require all inheriting classes to be in the same compilation unit as the declaration and to be themselves marked as either closed or sealed. Interestingly, this can be easily extended for interfaces as well, where closed would imply that the implementing type would have to exist in the same compilation unit. This would encompass the previously proposed InternalImplementationOnly attribute without adding any new concepts to the language.

The main issue with this simple addition is that it would only be enforced in the compiler, probably by the presence of a custom attribute in metadata. This would mean that any language which doesn't understand closed, or even any previous version of the C# language, would not be under any constraints in implementing these types. Unfortunately, this causes significant issues with any language using closed as it would generally be seen to be a guarantee of the possible subtypes or implementations, when in fact any non-participating language would violate those assumptions.

However, attempting to encode the objects in such a way as to use the .NET object system to restrict usage has an unfortunate set of problems that I'll detail here.

Issues with using .NET to encode closed

The first barrier is that .NET provides no method of restricting interface implementation, so applying closed to interfaces is impossible.

The second barrier is that .NET only provides a way of restricting inheritance of objects by limiting the visibility of a constructor. If an object is visible outside its declaring assembly, all the constructs must be marked internal or stricter in order to prevent inheritance outside the declaring compilation. If InternalsVisibleTo is in play, things become even more complicated -- the only way to prevent inheritance is to mark all constructors private and require that all classes inheriting from the closed class be inner classes. Assuming we don't want to reify that encoding into the language, this would require lowering definitions and usages to the appropriate class layout.

Requiring all classes to be sealed or only constructed inside the declaring compilation is a heavy restriction, but that's not the only limit. There's at least one other restriction in the C# type system: classes which do not have public default constructors cannot be used as parameters to a type parameter with the constraint T: new(). If we go down this path, there may be other complications in the same vein.

Now that I've listed the issues with using the type system, I'd like to discuss why not using the type system isn't as bad as it first seems.

Failure scenarios

In this instance, "failure" is defined as a language which supports and understands the closed modifier encountering a type which should have been prohibited by the existence of closed. There are only a few instances where this is the case:

  1. Compilation A defines type A, which is marked as closed. Compilation B, which is written using a language which does not understand closed, creates subtype B, which inherits from type A. This type is then passed back into compilation A via a public API. In this instance, we can see the compilation B is interacting with compilation A as both a consumer and a provider through its public API. The violation of compilation A's public contract is the underlying cause of the failure, regardless of the support or lack thereof from the language. Many public APIs have contracts which cannot be expressed in the type system, like various argument out of range conditions, but this is not often seen as an unredeemable flaw in the language design. In addition, like out of range conditions, closed type violations could be checked at runtime at public API boundaries to provide early feedback and failure for misuse of the API.
  2. Compilation A defines type A, which is marked as closed. Compilation B, which is written using a language which does not understand closed, creates subtype B, which inherits from type A. Compilation C, which supports closed, references compilation A and is referenced by compilation B. Type B is passed from compilation B to compilation C, where it proceeds to fail inside compilation C. Unfortunately, in this case the failure in compilation C is more of a violation of the contract of compilation A rather than compilation C, so efforts to shield against failures in compilation C is difficult and, in a sense, inappropriate.
  3. Other failures are transitive failures of the above, for example a tertiary compilation passing a "bad" value into a compilation, which then passes it through to a failing compilation.

(1) is the case I believe is the least complicated and would be the most common. I also believe it is the case least worth worrying about. In addition to being similar to other misuses of a public API, the failure is caused directly by a compilation creating an invalid value based on a referenced API, then passing that invalid value back into the API. In this case 1) the constructor of the bad value should have been aware of the public contract of the referenced assembly because it is both consuming, augmenting, and utilizing it directly, and 2) the referenced API has the greatest ability to be "defensive" in its consumption.

The case in (2) is a bit more complex, both because the contract may be more implicit and the failing compilation may be less able to protect itself. However, that doesn't mean that the risk is completely unlimited. For one, compilations can limit their public API to not rely on types marked as closed. Also, the situation becomes more pathological as the compilation references and API contracts become more complicated, but they also become rarer. Overall, it seems that this failure mode is worth the significant benefit it provides.

Conclusion

There are no trivial wins here, but it appears that a simple custom-attribute-enforced closed type modifier would provide substantial benefit that may outweigh the downsides. The only thing that seems clear is that attempting to provide a .NET-object guarantee with arbitrary closed hierarchies can get very complicated very quickly and is probably not worth the complexity and limitations it brings.

@DavidArno
Copy link

"Providing an ADT-like pattern is helpful for the pattern matching case but it unfortunately doesn't integrate well with current object-oriented patterns written in C#"

I'm not convinced that pattern matching will ever fit well with existing OO designs. It's a functional construct and thus surely it would be better to use records and discriminated unions with pattern matching? It's an opportunity to improve existing designs.

@alrz
Copy link
Member

alrz commented Feb 16, 2016

@DavidArno Agreed, but it doesn't mean that you shouldn't be able to use pattern matching with existing language constructs. If that wasn't the case, you would need to do extensive refactoring just to be able to use it in your code. I think the proposal is trying to fill this gap.

@bondsbw
Copy link

bondsbw commented Feb 16, 2016

So is part of this proposal that all record types implicitly use the closed modifier and its semantics (including how the result is emitted in IL)?

Is it possible to extend this concept to virtual properties and methods?

@agocke
Copy link
Member Author

agocke commented Feb 16, 2016

@bondsbw Record types could use this proposal or not. I'm not declaring anything either way right now, but I would expect some synergy here.

As for virtual members, theoretically this could be applied individually (presumably without the class being itself closed, because that would just be redundant) but that seems like a strange, possibly bad API design. I'd have to see a real use case that would benefit before applying the modifiers to members.

@agocke
Copy link
Member Author

agocke commented Feb 16, 2016

/cc @gafter @MadsTorgersen

@bondsbw
Copy link

bondsbw commented Feb 17, 2016

@agocke Consider this example, where both classes are in the same compilation unit:

public class Worker { 
    public closed virtual bool IsBenefitsEligible {
        get { return false; }
    }
}

public class Employee : Worker { 
    public bool IsFullTime { get; set; }

    public sealed override bool IsBenefitsEligible {
        get { return IsFullTime; }
    }
}

This enforces the semantics that in order to be eligible for benefits, a worker must be a full-time employee. No externally derived classes are allowed to change that.

@agocke
Copy link
Member Author

agocke commented Feb 17, 2016

@bondsbw Sure, the sealed was useful here, but how was closed useful? I would need a real-life example from a library why it would be useful to restrict overriding of specific virtual methods.

@bondsbw
Copy link

bondsbw commented Feb 17, 2016

@agocke closed prevents an external class derived from Worker from overriding IsBenefitsEligible, doesn't it?

Without closed, an external class derived from Worker could change whether the worker was considered eligible for benefits (which, for the sake of argument, is not allowed according to our requirements).

@agocke
Copy link
Member Author

agocke commented Feb 17, 2016

@bondsbw Right, I'm arguing that coming up with that arbitrary requirement isn't good enough. I need to see an actual reason why someone would have such a requirement and it wouldn't be considered bad design.

In the Worker/Employee case I think it would be reasonable to mark Worker as closed, but I don't see why the IsBenefitsEligible member would deserve special casing.

@HaloFour
Copy link

@agocke Regardless of the syntax how is it proposed that this would be enforced? It seems to be something that would require CLR support given that there's really nothing that the compiler could do to prevent a non-sealed publicly create-able class from also being used as a base class.

I like the idea in #8726, although it is more of a CLR proposal than a compiler proposal. Basically it would allow a type to be sealed except for a given white-list of derivable types. Similar to InternalsVisibleToAttribute in that it allows for relaxing the rules but only in that specific circumstance.

@bondsbw
Copy link

bondsbw commented Feb 17, 2016

@agocke IRS rules state that there are significant tax differences based on whether a worker is considered an employee, or not. Thus, the requirement that two classes exist, workers who are employees and workers who are not. And many companies only allow employees that are considered full time to be eligible for a benefits package. Both of these requirements are quite common in the real world, and in our hypothetical company we are requiring that such classifications be used strictly.

Now, why can't Worker and Employee be closed? Because we still want to allow users of our library to derive those further, like any normal class:

Example 1) A subsidiary creates classes ExemptEmployee vs. NonExemptEmployee, which determine overtime eligibility according to the Fair Labor Standards Act.

Example 2) Your company contracts out transportation duties, and in this situation the classes IntrastateDriver vs. InterstateDriver align best with motor carrier laws.

Example 3) Your business unit is better served with employee classes Developer, Analyst, ProjectManager, QAEngineer, and so on.

@agocke
Copy link
Member Author

agocke commented Feb 17, 2016

@bondsbw Seems reasonable. I also like the symmetry with the exiting sealed modifier.

@gafter
Copy link
Member

gafter commented Feb 19, 2016

One of the advantages of the compiler knowing that a hierarchy is closed is that the compiler can produce a "kind" field that it can use to switch more efficiently among the various subtypes, as we do extensively in the Roslyn code base. The proposed approach here doesn't seem like it is a very neat way to accomplish that. How would the compiler importing an assembly that used this feature know how to use the kind field?

@alrz
Copy link
Member

alrz commented Feb 19, 2016

@agocke TypeSymbol that you've mentioned is internal already, then couldn't it be considered as a complete pattern with #8455? However, it wouldn't have the efficiency of ADTs (#6739) like what @gafter described. And since closed actually needs CLR support what does it buy you beyond ADTs? For example, with this much flexibility that they could provide.

@agocke
Copy link
Member Author

agocke commented Feb 19, 2016

@gafter You are correct, this would be no more efficient -- generating a type field would be difficult if not impossible if based solely on this feature. For that use case I think we would require proper ADT support.

@alrz TypeSymbol is internal, but Roslyn has IVT to a number of assemblies. The proposal also clearly explains how to do this without CLR support and the drawbacks of doing so.

@gafter
Copy link
Member

gafter commented Apr 21, 2017

Issue moved to dotnet/csharplang #485 via ZenHub

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants