[Review] SE-0117: Default classes to be non-subclassable publicly

-1

This proposal makes Swift a more confusing language.

Swift already has a mechanism for creating public subclassable classes and non-subclassable classes:

  public class SubclassableParentClass { }

  public final class NonSubclassableParentClass { }

This mechanism also applies to methods, properties, and subscripts:

  public func bar() {}

  public final func foo() {}

The proposal makes no effort to remove this existing syntax.

The very fact that this would be legitimate syntax as a result is a bad omen to me:

  subclassable final class ConfusedParentClass {

    overridable final func quuz() {}

  }

The proposal doesn’t even address what that would do, the obvious answer is “compiler error,” but a better answer would be a language design that didn’t allow for this kind of ambiguity.

Conflating access control and finality is confusing. The proposal actually even goes as far to argue that—“conflates” is a word I took from the proposal—but it’s solution *is* a conflation in of its right, because the only way to explain the results is in terms of both:

classes, methods, properties, and subscripts with access control of `internal`, `file private`, and `private` are overridable by code that can access them, to prevent this add the `final` keyword.
classes with access control of `public` are not overridable by code that can access them, to allow this replace the `public` keyword with the `subclassable` keyword.
methods, properties, and subscripts with access control of `public` are not overridable by code that can access them, to allow this replace the `public` keyword with the `overridable` keyword.

Not only is this complicated, and confusing, it isn’t even consistent: to deny overriding or subclassing you add the same keyword; but to allow overriding or subclassing you replace one keyword with two different ones, depending on which you’re doing.

I agree that the alternative of flipping the default, and replacing `final` with `nonfinal` is also undesirable. One of the nicer features of the Swift language design is that the language is easiest for app developers working within a single module, where it can be assumed that “everyone is an adult.” Breaking this to support the less common case of Public API Designers would be a step backwards; their case is important, but it shouldn’t come at a penalty.

Scott

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

        https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

        * What is your evaluation of the proposal?

+1. Being able to control how a class from my libraries are going to
be used by third-parties could enable a better designed API with more
control of how it is intended to be used. I'm just not fond of using
the proposed keywords ('subclassable' and 'overridable') as they feel
more like protocol or attribute names; I'd be more inclined to use the
alternative 'public open' instead, or 'public(open)' as a second
option.

        * Is the problem being addressed significant enough to warrant a change to Swift?

I'd say it is significant to every language.

        * Does this proposal fit well with the feel and direction of Swift?

Yes.

        * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

C# uses the keyword 'virtual' to explicitly mark methods that can be
overriden (not considered in the alternatives but I'm not a big fan of
it).

        * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I've took (a small) part on the thread discussing this proposal but
followed it closely

  * What is your evaluation of the proposal?

Strong +1 with the modifications proposed by Brent, I think that a single, short keyword is preferable to two different ones, and `open` is a perfect candidate that express its intent very well.

The only question that it raises is, a public class can be extended with a protocol defined in another module? I think that this aspect is not addresses in the proposal, or I have missed it completely.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes

  * Does this proposal fit well with the feel and direction of Swift?

Yes indeed! This will fit nicely with the secure aspect of the Swift language, and will avoid cases where the library authors will try to discourage subclassing inside the comments in the class declaration file.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

N/A

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I’ve read the proposal

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

I posted this somewhere else in another thread, but I still think a slightly different approach to all of the access namings might make more sense:

- everything is “sealed" within its module by default, no keyword for this

- use “export” (formerly “public”) to mean “visible outside of the module, no subclassing or overriding externally, but overriding and subclasses allowed internally unless otherwise restricted”

- use “export(public)” to mean “visible outside of the module, can be subclassed or overridden externally”

- “export final” means that the class/method is also final within its own module

- using “export(public) final” *probably* doesn’t make sense, but would mean overriding/subclasses allowed externally, but not internally

My argument for “export” here is that it is the logical inverse of “import” which is how you get another module’s stuff into your module in the first place.

Then I would want to consider a rename of “internal” to “public” and “fileprivate” to “internal” so the complete list would be:

export - accessible outside of the module
public - accessible anywhere within the module
private - accessible only within the scope
internal - accessible within the same source file

In this way, there is nice symmetry between import/export, and then again between public/private. The source-file-based grouping of “internal” now has a name that is more unique and not immediately related to the other access keywords, either, which makes sense since it’s on a slightly different axis.

After all of this, the “export(public)” case rational would seem to be logical - you’re saying that the symbol is accessible outside of *this* module and subclassable/overridable within the “public” context of whatever module is importing it - hence using both “export” and “public”.

So anyway, I don’t know if this counts as a -1 vote or not or if this is even coherent, but this is my feedback at the moment. :slight_smile:

l8r
Sean

* What is your evaluation of the proposal?

+1, with bikeshedding.

To me, “subclassable” does not imply “public.” If I hadn’t read this proposal, I would be likely to declare an internal class subclassable, and then be shocked when it was exposed as public!

If it’s part of my public API, I want to see the word “public” on it. That should not be implicit under any circumstances. I strongly prefer any one of these to the standalone “subclassable:”

  • public(subclassable)
  • public(open)
  • public(extensible)

The extra verbosity is tolerable because subclassability by design is (should be!) the exception, not the rule.

* Does this proposal fit well with the feel and direction of Swift?

Yes, it fits two patterns of Swift’s general design:

if one choice is the default, it is the safer one; and
consequential design decisions are explicit. Implicitness is reserved for obvious / redundant information that becomes noise and interferes with readability.

To that end…

* Is the problem being addressed significant enough to warrant a change to Swift?

…open public classes run against general precedent in the language, so yes.

As programmers grow used to Swift, they will increasingly lean on the compiler to prompt them when there is an important type design issue to consider, or when they are doing something that compromises type safety. The absence of a compiler warning becomes important information.

This means that making open classes the default is not just aesthetically suboptimal; it is actively misleading.

* If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No. In all the OO languages I’ve used, classes are open by default. That raises a question: is this really a good idea? Aren’t we ignoring established precedent? What about all the unauthorized subclassing we’ve done in the past?

I’m sympathetic to those accustomed to Objective-C, Ruby, Javascript, Python, and other languages with more fungible type systems who argue that it’s unreasonable to expect us all to use classes only as library authors intend. Monkey patching has saved many a day, they’d say. And it’s true, it has! I rely on it.

My counterargument: the ecosystem is changing.

In the era of increased open sourcing, easy forking, and more community-driven development, this concern is less severe than it used to be. I rarely use any closed-sourced libraries for iOS development. If I need to tweak some library and non-subclassibility is getting in the way, then I can fork it — and perhaps even contribute my changes back to improve the upstream project. In an open source world, “closed by default” makes a lot more sense.

The big exception to this is Apple itself, and the direction of this proposal implies a large cultural shift in how Apple deals with its developer community. Having the Swift language so aggressively prevent the dubious, brittle, bad-idea-but-it-gets-the-job-done workarounds that the Obj-C runtime allowed means that Apple’s platforms are going to have to be more consciously extensible, more transparent in their design decisions, and more responsive to unanticipated needs. This community right here is an exemplar.

Here’s thing: this big shift goes far, far beyond this proposal. Swift already rules out most any form of messing with a library’s assumptions about its own design. With or without the proposal, Apple is already getting on board with the new regime of stricter typing and more hermetically sealed libraries. They’re already going to have to make API design decisions about how and where extensibility is allowed with unprecedented caution.

This proposal hardly alters that tipping balance; it just removes a design inconsistency.

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I read the proposal carefully, the discussion thread not at all.

Cheers,

Paul

-1, against all odds.

I can't fight the feeling that many fans of ideas like this believe that good designed libraries come for free by simply adding restrictions — which, at least in my opinion, just isn't true:
No matter what the defaults are, good libraries are hard to build, so I predict this proposal would not only fail in increasing framework quality, but also will make it much harder for users of those frameworks to work around their flaws, which are just a natural part of every software.

The change would be less painful than final by default, and most normal developers wouldn't have to suffer (at least with their own code), but their is still confusion to expect:
When you tell someone about a "subclassable" modifier, the natural expectation is that this is needed for every class you want to subclass...

It's similar with overridable, but additionally, this would destroy a possible way to annotate a method that can be overridden, but not called, which I'd consider as quite useful.
So, at least I vote for a modification that overridable doesn't imply public — not only because it's more powerful, but also because implying things is a complication

  * Is the problem being addressed significant enough to warrant a change to Swift?

no, I question there is a problem with the status quo at all

  * Does this proposal fit well with the feel and direction of Swift?

(does anyone ever write something here that contradicts his own evaluation? :wink:

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Not exactly the same, but C++ with its "virtual" is similar (C++ at its time had the excuse of better performance for the inconvenience; I don't think this small benefit should be important for Swift)

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Just read the proposal and the messages, and took part in similar discussions before

Tino

  * What is your evaluation of the proposal?

Not sure. If we want to be clear about what can be done with public class/method outside the module, and I agree this is an important question, IMO we should to force a developer to explicitly mark the class/method as 'public&final' or as 'public&open'. I.e. the developer of module/framework should explicitly decide if he/she wants or don't want to allow for subclassable/overridable.

If one writes 'public class' - does this means that he/she *decided* to not allow subclasses for this class or just *forgot* to decide this?

Also, IMO the suggested 'subclassable class' / 'overridable func' has no relations to access scope but to ability for extension of the class. So these keywords just should not replace the 'public', but should be used in conjunction.

So, my -1 to proposed 'subclassable'\'overridable' as *replacement* for 'public'. They confuse and not clear if it is public or modifiers for 'internal' scope.
My +1 to alternatives like 'public(subclassable)' or 'public subclassable'

My 0.0 for the idea of default non-subclassable `public`. I believe it is important for developer to decide what should be allowed to his/her public class, so Swift should force the developer to explicitely decide this.
My +1 to require 'public(final)' or 'public(subclassable)', not just 'public'.

···

On 06.07.2016 2:11, Chris Lattner via swift-evolution wrote:

  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Hello Swift community,

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

   https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

   * What is your evaluation of the proposal?

-1

Perhaps because this is so different from what I have seen in other languages and used for so many years. I have not worked with a language that uses non-subclassable/ non-overridable as the default.

I think that by default classes should be subclassable, not the other way around. I am afraid that developers will not take the time to specify which methods are overridable resulting in libraries that are difficult to patch, extend.

In my 26+ years of object-oriented design and programming (other languages, Objective-C since 1990 and Java since 2001) I have worked with object oriented libraries and subclassed methods that the authors probably never anticipated. I have been able to fix problems, enhance classes by creating subclasses with fixes and enhanced behavior.

In java for example I have seen that sometimes I would have been able to fix bugs or enhance the existing classes had the author not chosen a method to be protected or private. Sometimes they had a good reason but sometimes they didn't. Is have been able to survive using an awesome library that was discontinued and end-of-lifed thanks to subclassing that has allowed me to fix problems and enhance over the years as the Java language kept evolving.

In general I like to design classes with methods that have a very well defined purpose / logic. Such methods are potentially overridable. I find that making a method private or final can be selfish / restrictive at times and I choose it carefully for implementation details that are better served by going through the public methods.

I think that making a class not subclassable by default is restrictive / limiting / selfish.

Sometimes the extension points are clear.
I also think that every other method with a well defined purpose / logic is also potentially an extension point.

In my experience we should allow the developer to override by default. That is how I design my classes and every method / property.

I use private for the stuff that is obvious that should not be exposed.

In the motivation section performance is also mentioned as driving this proposal. However I don't see any study that supports that. I would like to see that. This should not be taken lightly.

Let's imagine that performance is important for a library that is heavily used and that the classes are not the type that you usually override. Wouldn't we be better served by being able to seal the class, i.e. "public sealed class Foo" and then for the methods / properties that are clear extension points should be flagged overridable. I would prefer something like that. And I think it would be more intuitive.

   * Is the problem being addressed significant enough to warrant a change to Swift?

No.

   * Does this proposal fit well with the feel and direction of Swift?

I think it is counter-intuitive. I don't think that reading "public class Foo" would make anyone think that Foo is non-subclassable.

On the other hand, reading "public class sealed Foo" would suggest to the reader that the class can be overridden but only the methods that are flagged as overridable.

If we wanted to prohibit overriding then we could use "public final class Foo" without any extension points. Then nobody would be able to subclass and it would be an error to try to flag a method / property as overridable.

   * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I don't recall having seen this behavior in the languages that I have worked with.

   * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I read the whole proposal and have been thinking about the implications for a few hours.

···

On Jul 5, 2016, at 7:11 PM, Chris Lattner <clattner@apple.com> wrote:

More information about the Swift evolution process is available at

   https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce

Hello Swift community,

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?

-1
I think it's a step backwards.
If we really think that Swift is meant as a language for beginners then this is certainly the wrong direction to go to. Having to use a special keyword to allow a class to be subclassed from outside the module looks like the compiler has to perform some extra effort to add this "subclassability feature".

It's not bad to allow sealing of classes. I don't see real value in it though. And `sealed` should not be the default. Either the class if final or not: for me that's a valuable distinction. "sealed vs. unsealed" is not.

The proposal is another try to prevent people from misusing the language. But misusing the language will always be possible and will always be easy. All these attempts will make the language just more complicated or harder to understand. I don't buy the performance argument either, i.e. that "sealed by default" will improve performance considerably for non-contrived use-cases. But I'm sure you will find that out a few months after the proposal is implemented :-/

  * Is the problem being addressed significant enough to warrant a change to Swift?

No

  * Does this proposal fit well with the feel and direction of Swift?

No, I don't think so.
If you have a ParentClass and a SubClass, and the ParentClass is sealed while the SubClass is subclassable. What happens? No matter how this question is answered, I don't like the answer. (compile error => bad. || make it as the user wishes => bad; what do we gain by letting ParentClass remain sealed? || make ParentClass implicitly subclassable too => bad.)

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

No

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Participated in early discussions which didn't convince me at all.

-Michael

···

Am 06.07.2016 um 01:11 schrieb Chris Lattner via swift-evolution <swift-evolution@swift.org>:

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

  * What is your evaluation of the proposal?

+1 (almost exclusively)

Writing good library code is hard and a lot of code isn't actually written to be externally subclasses (and trying to plan for subclassing can be difficult). Our team also writes "final class" in a lot of places and I don't like it (but wouldn't want final by default either).

This proposal feels like a sensible trade off to me. Having to intentionally mark classes for public subclassability leads to better safety (only classes being intended for external subclassing actually being subclassable), clarity (by documenting what's designed for subclassing/overriding), performance (by enabling optimizations if a class isn't subclasses internally) while still providing convenience within the module (enabling the authoring team to subclass and not having to put final throughout).

There is some negative with regards to limiting the ability to patch third party code, but I think the benefits of this proposal are worth that.

The behavior of Objective-C classes and @testablity are an important part of this proposal.

Bike sheding:

The subclassable/overridable pair read well on their own but it's not obvious to me that subclassable is also public and that it's not needed internally. This could be helped by requiring "subclassable" classes to also be marked "public" and making it an error to declare a class as "internal subclassable" or "private subclassable" (all 3 could have a simple fix-it).

I would also be fine with either "open" or "extensible" (although I could see "extensible" being interpreted as "can have extensions" instead of "can be subclasses").

   * Is the problem being addressed significant enough to warrant a change to Swift?
   * Does this proposal fit well with the feel and direction of Swift?

Yes. Being safe by default is well aligned with Swift. I would also argue that the same applies to being explicit about the authors intentions where it's needed.

   * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

None. Objective-C is completely open and I can't recall any other language that differentiates between "public" and "public subclassable".

   * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Read the proposal and the review feedback so far.

- David

···

On 6 Jul 2016, at 01:11, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

   https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

   https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

   * What is your evaluation of the proposal?
   * Is the problem being addressed significant enough to warrant a change to Swift?
   * Does this proposal fit well with the feel and direction of Swift?
   * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
   * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

   https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

* What is your evaluation of the proposal?

I’m on board with solving the problems outlined here. In particular, continuing to make Swift safe and fast by default and extending access and features as needed. And I am mostly on board with the solution proposed here.

I have 2 big concerns:

1) Unlike most changes to the language, the problems from this change will not be immediately evident. While a module author typically uses their module in at least 1 project of their own, they don’t see the usage point for most of their API. This change will likely cause a long process of updates for modules going back and forth between updating the module and the app that uses it.

2) As the proposal is written, we are introducing 2 new terms into the language that are not that different from what we already have (final and public/internal/private). I would really like to see these terms merged to limit the vocabulary of the language. Personally I am in favor of public(extensible) for both classes and methods.

* Is the problem being addressed significant enough to warrant a change to Swift?

Yes. In particular, giving the language the ability to fine tune what can and cannot be overridden is very helpful.

* Does this proposal fit well with the feel and direction of Swift?

Yes. The biggest argument I’ve seen against this behavior is that we need to be able to fix Apple’s frameworks for them. But the direction of Swift is away from doing this (having final available at all, no method swizzling etc.) and for good reason.

* If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

There was an interesting discussion somewhat related to this on the latest Bike Shed (http://bikeshed.fm/70) coming from the perspective of Ruby developers.

* How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I’ve read through the final proposal and have also given it occasional thought over the weeks building up to the proposal. I’ve also read a handful of responses.

David Beck
http://davidbeck.co
http://twitter.com/davbeck
http://facebook.com/davbeck

Hello Swift community,

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

  https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

  * What is your evaluation of the proposal?

I think the goals are laudable but a -1 to the proposal as written. It conflates different things and magically having different behavior inside and outside the module. It seems to add a higher cognitive burden than is really necessary… I imagine trying to explain the behavior to a Swift newcomer and it never being clear on the first attempt. Subclassability is a separate concern from visibility, plus final doesn’t cleanly compose with this design.

IMHO It makes no sense to have a member that is public, non-virtual to external consumers, but overridable to internal consumers. If that’s the case, why not just use a protocol to completely hide the type? It just doesn’t seem like a common case to me.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

C# makes members non-virtual by default, so visibility naturally controls who can see the type and virtual controls subclassing/overriding. Of course C# has protected, which controls visibility to subclassers so you can mark things as internal protected virtual, then the overall type as public virtual. Someone can still subclass but it doesn’t matter because they can’t override anything as all the protected override points are internal… not that it was very common!

When you’re designing things for easy subclassing I found it much more useful to offer protected members as the override points without letting subclassers override the public interface. Much less opportunity for them to screw things up, forget to call super, etc. In the rare case where I wanted to allow them to skip super’s implementation I just added that as a parameter to the protected member.

Russ

···

On Jul 5, 2016, at 4:11 PM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The review of "SE-0117: Default classes to be non-subclassable publicly"
begins now and runs through July 11. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

Reviews are an important part of the Swift evolution process. All reviews
should be sent to the swift-evolution mailing list at

        https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the
review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review
through constructive criticism and contribute to the direction of Swift.
When writing your review, here are some questions you might want to answer
in your review:

        * What is your evaluation of the proposal?

A strong +1 to the new default. I agree with others that having two
keywords that also conflate/imply public access is less than ideal. Having
a `public(open) class` or `public open class` would be better in my opinion.

        * Is the problem being addressed significant enough to warrant a
change to Swift?

Yes.

        * Does this proposal fit well with the feel and direction of Swift?

Yes. Requiring extra keywords and safety at module boundaries to clarify
intent has been a philosophy toward Swift language changes.

        * If you have used other languages or libraries with a similar
feature, how do you feel that this proposal compares to those?

N/A

        * How much effort did you put into your review? A glance, a quick
reading, or an in-depth study?

Read the proposal as well as this and earlier discussions.

···

On Tue, Jul 5, 2016 at 7:11 PM, Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

        https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Trent Nadeau

Hello Swift community,

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?

-1

In general terms, the philosophy “you are not allowed to do stuff with my classes unless I say so” is repugnant to me. Also, the breakages in existing code would be more serious than usual.

The specifics:

Firstly, making a class public does not provide “two different” capabilities, it provides one: the class is now accessible outside the module. Using a class includes instantiating it, calling functions, accessing properties and also subclassing non final classes and methods. If you choose to make a method or class public you are already having to carefully consider its design perhaps adding guards for parameters validationetc. In this context, adding final is not really onerous.

Secondly, if you don’t want to go to the trouble of making your class work properly, you already have the option of making it final.

Thirdly, this is a huge breaking change. Not only is it a breaking change, but for a lot of people, the breakages will be outside of their control. Consider if I publish a module with a class with public methods and you subclass it in your code. Once this change is implemented, my code will still compile and pass its unit tests but your code is now broken and you are dependent on me changing my code to fix your code.

Fourthly, the optimisation point is a red herring. Premature optimisation is the root of all evil, they say. This is premature optimisation. In any program, the number of methods where eliminating dynamic dispatch would provide a visible benefit is tiny. In those cases, you can add the word final to the declaration (after proper performance testing).

  * Is the problem being addressed significant enough to warrant a change to Swift?

The functionality to stop people subclassing and overriding where it would be dangerous to do so already exists and this coupled with the serious problem of breaking code where the fix is outside the developer’s control makes it a very bad change indeed.

  * Does this proposal fit well with the feel and direction of Swift?

No. Swift feels to me like a really nice powerful language that is easy to program in. Hopefully the direction of Swift is more of the same. This change is going the other way.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

I have used many languages with classes with inheritance: Java, C++, Python, Objective-C, C#, Smalltalk. They all more or less follow the current Swift convention (in C++ you had to explicitly say if a method was virtual and that could be quite confusing).

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I read the proposal and some of the other responses.

···

On 6 Jul 2016, at 00:11, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

Chris Lattner wrote:

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

   https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

   * What is your evaluation of the proposal?

+0.9. I'm positive with making the subclassability of exported classes explicit, but I'd reconsider the keywords proposed.

Given how the ecosystem has shaped up so far, there are very few cases where Swift classes could be realistically patched by subclassing and overriding. Instead, forking and recontributing open-source code is the better way to go. From my OOP in any programming languages, almost all unplanned (by library author) uses of subclassing have become terrible hacks that were hard to maintain in application code. And I don't want to imagine having to maintain a library that multiple users have patched by subclassing without a documentation of the contract between the superclass and its subclasses.

In Swift, classes seem to be most essentially about reference semantics and not so much about inheritance. With that, I think it's better that unlimited subclassing becomes a conscious choice by the library author, so that we also increase the probability of remembering to document what can be overridden and how. Originally, I would've even supported making `final` the default, but adding the new "sealed" inheritance mode is a very good compromise that makes life easier when writing application code.

Sealed classes also open a new language design opportunity where `switch` cases can be proven total given a sealed class type as argument.

However, I also think we shouldn't merge the the `public` modifier with the newly proposed keywords. From the alternatives mentioned so far, I'd give my +1 to `public open class/func` or the spelling `public(open) class`. In this position, I think it'll be unambiguous given context whether `open` acts as a keyword or not.

Alternatively, we could reuse the `super` keyword in this context; the only minor issue I can see in `public super class` (besides that "superclass" should be written without a space) is that public unsealed classes aren't the only publicly visible superclasses (sealed classes are also publicly superclasses of their public subclasses). Well, and that the "superfunc" in `public super func` would be an unfamiliar term, even if it's easily taught as the kind of function which you should consider calling as `super.foo(...)` somewhere in an override.

The proposal also left open whether the subclassability is inherited in subclasses. I think it's clear that the answer is "no", and thus every subclass of a publicly subclassable superclass is only publicly subclassable if explicitly marked so.

   * Is the problem being addressed significant enough to warrant a change to Swift?

Yes, Swift should use language design opportunities like this to statically encourage thoughtful library design.

   * Does this proposal fit well with the feel and direction of Swift?

Yes. It also doesn't take away the option yo make well-thought use of OOP better in the future, such as abstract classes.

   * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

N/A.

   * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Quick reading or a bit more.

— Pyry

  * What is your evaluation of the proposal?

Strong +1. I believe supporting public inheritance is the single most complicated thing in modern API design; thus, allowing inheritance to happen without explicit approval of the API designer is clearly a bad idea.

I'm OK with the proposed keywords (subclassable/overridable), but I like "open" even more.
I think "base class"/"base func" or "super class"/"super func" would also read well.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes. This proposal helps third-party API writers avoid a major source of pitfalls.

  * Does this proposal fit well with the feel and direction of Swift?

Absolutely. Having sensible/safe defaults for such toggles feels like a major feature in Swift.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Java is a huge poster child for object oriented programming, which is often misunderstood to be primarily about inheritance. Still, collections of Java best practices invariably include strongly worded advice for preferring composition over inheritance, and I especially like the following rule:

  Design and document for inheritance or else prohibit it.
      -- Joshua Bloch: Effective Java. Addison-Wesley, 2001.

Even in Java, it is a bad idea to leave classes subclassable; but having to remember to add final is a chore.

SE-0117 takes this a step further by allowing package writers to use inheritance internally when it makes sense, without also having to take on the complications arising from allowing third-party inheritance -- such as having to write a reasonably complete unit test suite for superclass-subclass interactions. This is an interesting improvement over final-by-default (which I'd also support).

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I carefully read it, drank a cup of my favorite beverage to celebrate its existence, then I collected my thoughts.

···

--
Karoly
@lorentey

On 2016-07-05 23:11:17 +0000, Chris Lattner via swift-evolution said:

Hello Swift community,

The review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 11. The proposal is available here:

https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md

Reviews are an important part of the Swift evolution process. All reviews should be sent to the swift-evolution mailing list at

  https://lists.swift.org/mailman/listinfo/swift-evolution

or, if you would like to keep your feedback private, directly to the review manager.

What goes into a review?

The goal of the review process is to improve the proposal under review through constructive criticism and contribute to the direction of Swift. When writing your review, here are some questions you might want to answer in your review:

  * What is your evaluation of the proposal?
  * Is the problem being addressed significant enough to warrant a change to Swift?
  * Does this proposal fit well with the feel and direction of Swift?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

More information about the Swift evolution process is available at

  https://github.com/apple/swift-evolution/blob/master/process.md

Thank you,

-Chris Lattner
Review Manager

_______________________________________________
swift-evolution mailing list
swift-evolution@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution

--
Károly
@lorentey

[Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md ]

John has done a tremendous job supporting this proposal; the position he’s articulated very closely matches mine. Thank you to both John and Javier.

I wanted to share a concrete use case that Daniel Dunbar relayed to me. He was working on a closed class hierarchy like the ones discussed here, where all of the subclasses are within a single module, but they are all public. The class also has a required initializer for dynamic construction, so that they could write something like this:

internal struct ModelContext { /*…*/ }

public class ModelBase {
  internal required init(context: ModelContext) { /*…*/ }
  // …
}
public class SimpleModel: ModelBase {
  internal required init(context: ModelContext) { /*…*/ }
}
public class MoreComplicatedModel: ModelBase { /*…*/ }

// (within some other type)
public func instantiateModelObject<Model: ModelBase>(_ type: Model) -> Model {
  return type.init(context: self.context)
}

That is, a public entry point calls a required initializer with an internal argument type. This is the only way to instantiate Model objects, and the internal context type doesn’t leak out into the public API.

Of course, Swift doesn’t allow this. If someone outside of the module subclasses ModelBase, there’s no way for them to provide the dynamically-dispatched 'init(context:)’, because they don’t have access to the internal ModelContext. The author of the library has to make the required initializers public, and either set the ModelContext separately or make it public as well. Even though no one outside the module should be using these APIs.

If ModelBase were public-but-not-subclassable, however, the code is perfectly fine. The initializer and the helper type don’t need to be public, and clients of the library see only what they need.

This is just one use case. I don’t want to say it’s a general model for everyone’s code. However, it does point to a desire for public-and-not-subclassable classes; any other solution would either require the library author making more things public, or the compiler making it possible to accidentally call an unimplemented initializer.

I’ll send a separate message with my thoughts as the primary author of the Library Evolution model, to keep those discussions distinct. That one will have a bit more ideology in it. :slight_smile:

Jordan

P.S. “Why not use protocols?” When a protocol has an initializer requirement, it still forces all subclasses of a conforming class to provide an implementation, i.e. the conforming class’s initializer still needs to be declared ‘required’. That means it’s subject to the same restriction: a required initializer must have as much access as the containing class.

  * What is your evaluation of the proposal?

-1

Once, for an extended portion of my career, I worked in a 'developer support' type role for a large C++ API. I had daily contact with developers who were consuming that API.
Because of that experience, I am fully convinced that library authors will never imagine all the ways developers will use a library's API.
Of course there's more to it, but I really think the reasons why this is the wrong direction have been well expressed by others.

  * Is the problem being addressed significant enough to warrant a change to Swift?

No, I don't believe so. I also think it would eventually force a second change; some sort of override to the default, at which point we've extended the language further to get closer to where we started.

  * Does this proposal fit well with the feel and direction of Swift?

I don't believe it does.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

N/A

  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

I have read the proposal, followed this discussion here but none of the previous discussion

Paul

  * What is your evaluation of the proposal?

Mixed bag.

I'm a big fan of sealed-by-default for classes. But I want a keyword (e.g. `sealed`) for this so I can be explicit about it too. Just as I sometimes mark methods as `internal` to make it clear to the reader that I didn't simply forget the access modifier, I would want to mark certain classes as `sealed` to make it obvious that I made a deliberate choice.

On the flip side, I'm very against sealed-by-default for functions. My general feeling is that, if my class is subclassable, marking certain public methods as non-overridable is the exception rather than the norm. Anecdotally, in the Postmates app, most of our Swift classes are explicitly marked as `final` (which supports sealed-by-default for classes), but for the subclassable classes, I'm pretty sure that _none_ of our methods are marked `final`.

Also, I think the keyword `subclassable` is kind of cumbersome. I'm much more in favor of `open`, or possibly `public(open)` since this keyword only applies to public classes (or public methods, if I'm overriding a sealed method from my parent).

And finally, I think we need to make the Swift migrator automatically mark public non-final declarations as `open`, otherwise we have a *huge* backwards compatibility hazard for all third-party Swift frameworks.

  * Is the problem being addressed significant enough to warrant a change to Swift?

Yes.

  * Does this proposal fit well with the feel and direction of Swift?

Yes. It's making things safer by default, and matches the default internal access level.

  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
  * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

A quick reading of the proposal, then reading part of this massive review thread and skimming the rest.

-Kevin Ballard

···

On Tue, Jul 5, 2016, at 04:11 PM, Chris Lattner wrote:

[Proposal: https://github.com/apple/swift-evolution/blob/master/proposals/0117-non-public-subclassable-by-default.md ]

(This is my second response to this proposal. The previous message shared a use case where public-but-non-subclassable made things work out much better with required initializers. This one has a bit more ideology in it.)

As many people have said already, this proposal is quite beneficial to library designers attempting to reason about their code, not just now but in the future as well. The model laid out in the Library Evolution document <http://jrose-apple.github.io/swift-library-evolution/> (often referred to as “resilience”) supports Swift libraries that want to preserve a stable binary and source interface.

In the Swift 2 model (and what’s currently described in that document), a public class must be final or non-final at the time it is published. It’s clearly not safe to add ‘final' in a later version of the library, because a client might already have a subclass; it’s also not safe to remove ‘final’ because existing clients may have been compiled assuming there are no subclasses.

(Of course, we can remove this optimization, and make ‘final’ a semantic contract only. I’m deliberately avoiding most discussion of performance, but in this parenthetical I’ll note that Swift makes it possible to write code that is slower than Objective-C. This is considered acceptable because the compiler can often optimize it for a particular call site. For those who want more information about the current implementation of some of Swift’s features, I suggest watching the “Understanding Swift Performance <https://developer.apple.com/videos/play/wwdc2016/416/>” talk from this year’s WWDC.)

With this proposal, a public class can be non-publicly-subclassable or publicly-subclassable. Once a class is publicly-subclassable (“open”), you can’t go back, of course. But a class that’s not initially open could become open in a future release of the library. All existing clients would already be equipped to deal with this, because there might be subclasses inside the library. On the other hand, the class can also be marked ‘final’, if the library author later realizes there will never be any subclasses and that both client authors and the compiler should know this.

One point that’s not covered in this proposal is whether making a class ‘open’ applies retroactively, i.e. if MagicLib 1.2 is the first version that makes the Magician class ‘open’, can clients deploy back to MagicLib 1.0 and expect their subclasses to work? My inclination is to say no; if it’s possible for a non-open method to be overridden in the future, a library author has to write their library as if it will be overridden now, and there’s no point in making it non-open in the first place. That would make ‘open’ a “versioned attribute <http://jrose-apple.github.io/swift-library-evolution/#publishing-versioned-api>” in the terminology of Library Evolution, whatever the syntax ends up being.

···

---

Okay, so why is this important?

It all comes down to reasoning about your program’s behavior. When you use a class, you’re relying on the documented behavior of that class. More concretely, the methods on the class have preconditions (“performSegue(withIdentifier:sender:) should not be called on a view controller that didn’t come from a storyboard”) and postconditions (“after calling loadViewIfNeeded(), the view controller’s view will be loaded”). When you call a method, you’re responsible for satisfying its preconditions so it can deliver on the postconditions.

I used UIViewController as an example, but it applies just as much to your own methods. When you call a method in your own module—maybe written by you, maybe by a coworker, maybe by an open source contributor—you’re expecting some particular behavior and output given the inputs and the current state of the program. That is, you just need to satisfy its preconditions so it can deliver on the postconditions. If it’s a method in your module, though, you might not have taken the trouble to formalize the preconditions and postconditions, since you can just go look at the implementation. Even if your expectations are violated, you’ll probably notice, because the conflict of understanding is within your own module.

Public overriding changes all this. While an overridable method may have particular preconditions and postconditions, it’s possible that the overrider will get that wrong, which means the library author can no longer reason about the behavior of their program. If they do a poor job documenting the preconditions and postconditions, the client and the library will almost certainly disagree about the expected behavior of a particular method, and the program won’t work correctly.

"Doesn’t a library author have to figure out the preconditions and postconditions for a method anyway when making it public?" Well, not to the same extent. It’s perfectly acceptable for a library author to document stronger preconditions and weaker postconditions than are strictly necessary. (Maybe 'performSegue(withIdentifier:sender:)’ has a mode that can work without storyboards, but UIKit isn’t promising that it will work.) When a library author lets people override their method, though, they're promising that the method will never be called with a weaker precondition than documented, and that nothing within their library will expect a stronger postcondition than documented.

(By the way, the way to look at overriding a method is the inverse of calling a method: you need to deliver on the postconditions, and you can assume the caller has satisfied the preconditions. If your understanding of those preconditions and postconditions is wrong, your program won’t work correctly, just like when you’re calling a method.)

This all goes double when a library author wants to release a new version of their library with different behavior. In order to make sure existing callers don’t break, they have to make sure all of the library’s documented preconditions are no stronger and postconditions are no weaker for public API. In order to make sure existing subclassers don’t break, they have to make sure all of the library’s documented preconditions are no weaker and postconditions are no stronger for overridable API.

(For a very concrete example of this, say you’re calling a method with the type '(Int?) -> Int’, and you’re passing nil. The new version of the library can’t decide to make the parameter non-optional or the return value optional, because that would break your code. Similarly, if you’re overriding a method with the type ‘(Int) -> Int?’, and returning nil, the new version of the library can’t decide to make the parameter optional or the return value non-optional, because that would break your code.)

So, "non-publicly-subclassable" is a way to ease the burden on a library author. They should be thinking about preconditions and postconditions in their program anyway, but not having to worry about all the things a client might do for a method that shouldn’t be overridden means they can actually reason about the behavior—and thus the correctness—of their own program, both now and for future releases.

---

I agree with several people on this thread that non-publicly-subclassable-by-default is the same idea as internal-by-default: it means that you have to explicitly decide to support a capability before clients can start relying on it, and you are very unlikely to do so by accident. The default is “safe” in that a library author can change their mind without breaking existing clients.

I agree with John that even today, the entry points that happen to be public in the types that happen to be public classes are unlikely to be good entry points for fixing bugs in someone else's library. Disallowing overriding these particular entry points when a client already can't override internal methods, methods on structs, methods that use internal types, or top-level functions doesn’t really seem like a loss to me.

Library design is important. Controlling the public interface of a library allows for better reasoning about the behavior of code, better security (i.e. better protection of user data), and better maintainability. And whether something can be overridden is part of that interface.

Thanks again to Javier and John for putting this proposal together.
Jordan

P.S. There’s also an argument to be made for public-but-not-conformable protocols, i.e. protocols that can be used in generics and as values outside of a module, but cannot be conformed to. This is important for many of the same reasons as it is for classes, and we’ve gotten a few requests for it. (While you can get a similar effect using an enum, that’s a little less natural for code reuse via protocol extensions.)

P.P.S. For those who will argue against “better security”, you’re correct: this doesn’t prevent an attack, and I don’t have much expertise in this area. However, I have talked to developers distributing binary frameworks (despite our warnings that it isn’t supported) who have asked us for various features to keep it from being easy.

Terms of Service

Privacy Policy

Cookie Policy