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

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:

  swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

  * 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:

swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

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

        swift-evolution/process.md at master · apple/swift-evolution · GitHub

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

  swift-evolution/process.md at master · apple/swift-evolution · GitHub

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:

   swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

   * 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:

swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

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

  swift-evolution/process.md at master · apple/swift-evolution · GitHub

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: swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub ]

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. :-)

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: swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub ]

(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/&gt; (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 <Understanding Swift Performance - WWDC16 - Videos - Apple Developer 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 <Library Evolution Support in Swift (“Resilience”) — Swift 3.0 documentation 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.

This has been a very interesting and educational thread.
Thanks to everyone who kindly replied to me and explained things.

Here is my 2¢…

* What is your evaluation of the proposal?

+0 as it stands…
+1 if there could be a reliable way for us to explicitly un-seal a sealed class for those times where its absolutely necessary.

If its at all feasible… maybe something like ...

public class MyWorkaroundClass : @forceOpen(YourSealedClass) { //Obligatory Compiler Warning Occurs
    public override func @forceOpen(brokenFunc()) {/*FIXME*/}
}
(The above is just a rough idea, please don't yell at me for the ugliness... :slightly_smiling_face:)

Just like we have forced unwrapping of optionals (for only when we really need it), I believe that we should be able to force unseal when we truly need it;

Another +1 if the keyword is changed from subclassable/overridable to something less confusing… since people might be easily be misled and think that even within thier own module if they don't mark it subclassable it won’t be subclassable… perhaps "open" is the better keyword…

public open class MyOpenClass {
    public open func openFunc() {/*OVERRIDE ME!*/}
}
In this case, "open" can only apply to public classes/methods/accessors, anything else using "open" ("private open" etc) will be an obvious error.

Please note that I am not against this proposal in theory; only that I would humbly request that the two items above be at least considered and I will happily jump on board.

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

I do think it is, because the potential upsides are many, such as better fencing between modules and users of those modules, potential performance gains and making it more explicit to a library writer what he/she is going to make public and also modifiable by consumers of the provided API.

Again though, I would be reticent to agree 100% if there werent a way to punch though that fence when we need to, not allowing such a thing makes the environment too rigid/unproductive and thats not the Swift I want to continue using. Swift is not C++ minus the C… is it?

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

I think overall it does because swift encourages more ad-hoc composition than inheritance based, so making inheritability more explicit does seem to fit in with the direction of Swift… although the lack of ability to escape that limitation just like we have with forced unwrapped optionals makes me feel its at the same time not fully in line with the direction of swift… hence the +0 instead of a full +1.

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

Someone here brought up Kotlin, but I have only dabbled with it.. It seems similar to that from what I can read… but I have no experience with writing or consuming a library in Kotlin.

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

I have been wrestling with this in my mind since late last year when it first appeared on the swift-evolution list as a general discussion.
I was originally very very strongly -1 on this (especially coming from an Objective-C background), but after reading the proposal and everyones replies both for and against, and thinking again deeply about my own code, have come around to believe that there are potential gains for this that are worthwhile, but only if some flexibility remains in the language to work around classes that are both closed-source and has a defect that is needed to workaround though subclassing.

···

——

Pros
Better Fencing Between Modules
Potential Performance Gains
Subclassing is Discouraged Unless its Explicitly Thought About

Cons
Unable to Override a Method/Accessor on a Class that Needs Fixing but is Closed Source
You are at the total mercy of the framework author… (this one is pretty big to be honest)
  
"Unknowns" that make it hard to judge...

What effect this will really have on the community…
What true objective performance benefits we can expect from inter-module optimization…
Will this really make libraries any better?

——

Other thoughts...
While it can be said that because most libraries are open source, the need to subclass is lower now than ever -we can just go in and fix something and send a pull request- those of us trapped in large corporations that have (usually poorly supported) SDKs shoved down our throats by the suits need a certain level of flexibility to work arounds things…

In debating to support this or not, I am reminded of comments by many here: Michael Tsai - Blog - Swift Proposal for Default Final

——

Thanks,

Andre

2016/07/06 8:11、Chris Lattner via swift-evolution <swift-evolution@swift.org> のメール:

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:

  swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

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

  swift-evolution/process.md at master · apple/swift-evolution · GitHub

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?
  * If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?

Extremely strong -1.

I understand the performance benefit, but it’s extremely problematic if you’ve used other languages like C++ that make the same tradeoff. The performance benefit also seems unnecessary for a lot of code, and if you need the performance, it’s easy enough to mark the class final.

Being able to subclass is critical not only to OOP design, but also Cocoa design. Making classes final by default will mean that a lot of classes will unnecessarily not support subclassing, which can complicate design.

This also strongly affects unit testing by getting rid of one of the easiest ways to mock input and output to a unit. It’s not enough to say third party code should be treated as black box. One of the best ways to test input and output from an object is to subclass the objects it communicates to and override the entry points. The biggest gaps I have in my code coverage are where I talk to non-virtual-by-default C++ methods and I can’t mock the output from those modules to hit all my cases.

As someone who writes an API for my day job, I am perfectly fine marking things as final by hand when necessary (when I finally get to jump to Swift.) It’s not inconvenient at all. I would much rather favor making public inheritance issues easier to surface in Swift. Public inheritance can be tricky, but this feels a bit like cutting off our noses to spite our own face. If you are a responsible public API vendor, you should be thinking about inheritance, or marking as final. And it’s wiping out one of Swift and Obj-C’s major advantages over C++.

The “final should be default because adding final after the fact is destructive” arguments are interesting, but ultimately not convincing to me. If you thought your class was safe for subclassing, but it ultimately wasn’t, this solves nothing. You’ll mark your class as subclassable, and you will ship it, and there will be issues, but it will still be too late to take things back. If you know your class is not safe for subclassing, you should mark it as final. There is no advantage here in that scenario.

This is all part of building a safe public API. Public API design can be difficult, but if you don’t understand safe subclassing designs, you are likely to miss the issues and mark your class as subclassable and ship it anyway. Again, the best way to tackle this is to find better ways to surface the issues. Making final the default still doesn’t solve the core issues of people not understanding the right design.

(I’d, ironically enough, be a lot more open to this proposal if Swift supported more dynamic things like proxies that could let you emulate subclassing without actually doing so. But I think that’s veering away from relatability of the language)

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:

swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

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. As a big proponent of API reviews for code that's going to end up being
used by others, I think sealed-by-default forces authors to think about how
their code might end up being used by others more so than more-permissive
alternatives. Seeing a keyword that marks a class as unsealed is a good
opportunity to start a conversation in a code review.

Given that Swift tries to encourage the use of value types over reference
types in many places, which can't be subclassed anyway, I don't think this
change should be a significant burden. I'm in the process of implementing a
fairly large library that's about 95% value types and 5% reference types.
The reference types are more of an implementation detail than a hierarchy
of types, so I would have them be final anyway. This is completely
anecdotal, of course, but it's a testament IMO to how differently Swift has
me thinking about the way I structure my APIs, and subclassing has been
mostly replaced by protocol extensions and composition.

I'm not compelled by the arguments that we need subclassing to fix bad
APIs, because that's absolutely not what subclassing is intended for. It's
a hack, and it's not a cure-all—it relies on being able to override the
correct methods and inject custom behavior in the first place. The argument
shouldn't be "we need open subclassing or we can't fix broken APIs"; that's
a false choice. It should be "we need *something* to fix broken APIs", and
that "something" should be proposed and the need for it should be argued in
its own right. The ability to fix a sealed broken API does have value, but
it is inherently unsafe because you might be making assumptions about
pre/post-conditions that the original class author didn't think about, and
those assumptions could become invalid in the future (or might rely on
internal implementation details that you cannot predict). Rather than
abusing a construct that is not suited for that purpose, those arguing for
the need for that functionality should propose something more appropriate.
This would be a win-win; we'd have a clean way of expressing API
boundaries, and an unsafe-but-acknowledged-as-such way of hooking into APIs
if a trap door is needed. (Of course, with Swift at such a young age, it's
hard to know what the needs for that are until we have some more mature
libraries to present as use cases.)

        * 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. The behavior that classes are unsealed within the same module but
sealed by default outside the module aligns nicely with the way internal
visibility already works in the language. Newcomers and app developers
don't have to think about it, but those who want to release code for others
to use must; I think that's the right level of responsibility.

        * 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?

Read the proposal and followed the heated discussion in the review thread.

···

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

More information about the Swift evolution process is available at

        swift-evolution/process.md at master · apple/swift-evolution · GitHub

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

-1 for this proposal, but +1 for solving the issues it raises

Regardless of what ends up being the defaults, I’m a very strong -1 on conflating visibility and subclassability/extendability.

- Dave Sweeris

···

On Jul 5, 2016, at 6: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.

+1 for the feature, +0.5 for being it the default and -1 for they syntax.
I'd prefer `open`, maybe `public(open)` or probably better `public open`.

I think library design is important and so this is a problem worth solving. I read the proposal and followed the discussion.

-Thorsten

···

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

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:

   swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

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

   swift-evolution/process.md at master · apple/swift-evolution · GitHub

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 think it's ultimately a good idea. Being noncommittal about subclassability/overridability—and thus forbidding it in public scope, but not making any promises about what the module does internally—is the alternative that preserves the most freedom for the module, and therefore the most appropriate default.

However, I don't like the `subclassable` and `overridable` keywords. They read like opposites of `final` with no implications for access level; I could easily imagine somebody marking an internal class `subclassable` assuming that it merely means it can be subclassed internally, and being very surprised (or even not noticing!) that the class has been made public. They're also long and cumbersome; that might be seen as a positive, but I think it will increase the inevitable backlash against this change.

I prefer the keyword `open`, which sounds like it could be a statement about the item's accessibility—and even sounds like it ought to be "more public than public"—and is short enough that it ought to be difficult to grumble about the change. It also means that both classes and members use the same keyword, and gives us a keyword that we can later use to "open" other things in the language, such as allowing you to extend enums with new cases.

Agreed; I also prefer "open" to having two different long keywords that don't (at least to my ear) imply "public".

I think Kevin Lundberg is right to worry about testability, but I don't think that has to prevent this change. Instead, we should permit `@testable` imports to subclass/override things that are not publicly subclassable/overridable, and thus a module built with "Enable Testability" on can't actually assume there are no subclasses/overrides of `internal` classes/members even if it doesn't see any. This will block optimizations in debug builds, but not in release builds. The proposal should be edited to explain this `@testable` behavior.

IIUC the basic design of @testable is to treat the tests for the testable thing as existing within its module, so I think this just falls out. I agree that it should be spelled out in the proposal, though.

John.

···

On Jul 5, 2016, at 6:45 PM, Brent Royal-Gordon via swift-evolution <swift-evolution@swift.org> wrote:

Extremely strong +1 from me on this proposal. It is the best default for many, many reasons (stated in the proposal and in Austin's review). It improves safety, facilitates and encourages reasoning about code, and will result in an ecosystem of overall higher quality.

···

Sent from my iPad

On Jul 5, 2016, at 6:49 PM, Austin Zheng via swift-evolution <swift-evolution@swift.org> wrote:

I'll bite.

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

        * What is your evaluation of the proposal?

Strong +1. I like this proposal because it forces programmers vending a public API to think about their extension points, and it also provides guarantees to consumers of library and framework APIs as to whether the framework developer intended for a particular class or member to serve as an extension point. More controversially, I like it because it trades off short-term subclass-based hacks in favor of a library ecosystem years down the line that will be significantly higher in quality than it would otherwise be - both because it'll be harder to write misbehaving code, and because it'll support an emerging culture in which API design merits more careful consideration than it would otherwise get.

I am largely unmoved by arguments involving Cocoa or misdesigned libraries: Apple framework engineers will annotate their frameworks however they want no matter what default we choose; the possibility of badly written code would argue against things like access control, static typing, 'noescape' by default, the lack of swizzling, or any of a huge number of features that could conceivably be used to patch misbehaving code. Developers working in the Apple ecosystem can always fall back to Objective-C as an escape hatch if they really need to monkey-patch Apple framework classes.

I have a question: one of the alternative syntaxes ("public(subclassable)") is listed as a potential candidate for expansion when resilience is implemented. Is there a description of a potential resilience syntax for the primary proposal?

        * 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?

Most definitely. For example, making closures non-escaping by default fits into the same model: expose only the most limited guarantees by default, with easy discoverability of more powerful guarantees should a framework author wish to use them. Another example is the raw pointer API proposal currently in the works. In this context, "limited" is not a liability, it is an asset: the more narrow the default semantics are, the easier it is for users to reason about the correctness of their code, and the more aggressively the compiler can optimize.

Indirectly, this proposal may also encourage framework authors to more carefully consider whether classes or protocols are better suited as extension points for their particular applications.

        * 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 read through the proposal carefully, and have read/participated in most of the threads on the topic over the past few months.

More information about the Swift evolution process is available at

        swift-evolution/process.md at master · apple/swift-evolution · GitHub

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

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

The problem is that it's very clear about what you're allowing—you're allowing `extension`s. Which of course isn't *actually* what you're allowing.

···

On Jul 5, 2016, at 7:16 PM, Karl via swift-evolution <swift-evolution@swift.org> wrote:

#1 - “extendable class” // explicitly clear about what I’m allowing

--
Brent Royal-Gordon
Architechies

Out of curiosity, what is your feeling about “internal” as the default level of access control? It seems that following your concern to its logical conclusion would lead to a design where all members of a public class would be forced to be public. After all, the author of a library or framework may not forsee the need to interact with a member that they did not explicitly mark public.

-Chris

···

On Jul 5, 2016, at 5:54 PM, Kevin Lundberg via swift-evolution <swift-evolution@swift.org> wrote:

* What is your evaluation of the proposal?

-1 as is. I do not want to be constrained by authors of libraries or
frameworks into interacting with a system in only the ways they forsee.
By making the default be non-subclassable, if a designer does not put
thought into all the ways a class can be used then I as a consumer of
the library am penalized.

+1. I'd prefer the "open" keyword though, and use it as a modifier of
public.

···

El mié., 6 jul. 2016 a las 1:24, Charlie Monroe via swift-evolution (< swift-evolution@swift.org>) escribió:

Huge +1.

Question about inheritance though:

class A {} // Not publicly subclassable
subclassable class B: A {} // Publicly subclassable
class C: B {} // Not publicly subclassable? Or is the subclassability
inherited?

I'm not a big fan of the subclassable keyword either since it's quite
long, not to mention it contains "class" which is the next keyword, making
it visually repetitive.

I'd prefer open on both class and func to introduce only one keyword
instead of two and possibly make it as a modifier of public to keep it in
line with private(set)...

> On Jul 6, 2016, at 1:11 AM, 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:
>
>
swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub
>
> 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
>
> swift-evolution/process.md at master · apple/swift-evolution · GitHub
>
> Thank you,
>
> -Chris Lattner
> Review Manager
>
> _______________________________________________
> swift-evolution mailing list
> swift-evolution@swift.org
> https://lists.swift.org/mailman/listinfo/swift-evolution

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

Huge +1.

Question about inheritance though:

class A {} // Not publicly subclassable
subclassable class B: A {} // Publicly subclassable
class C: B {} // Not publicly subclassable? Or is the subclassability inherited?

This is a great question. There are really two questions:

Can B be marked "subclassable" even though its superclass isn't? Allowing this allows creation of new classes with A as an ancestor. Since this is explicitly opt-in all within the same module I'm inclined to say we should allow it.

Is subclassability inherited by in-module subclasses? This seems to me opposed to the spirit of closed by default. I think it is better to require annotation on *every* class an method that is open to extension outside the module, regardless of what choice a superclass makes,

This gives module authors the most control over *exactly* which classes can be used as superclasses outside the module (A and C don't need to be subclassable just because B needs to be.

-Matthew

···

Sent from my iPad

On Jul 6, 2016, at 12:24 AM, Charlie Monroe via swift-evolution <swift-evolution@swift.org> wrote:

I'm not a big fan of the subclassable keyword either since it's quite long, not to mention it contains "class" which is the next keyword, making it visually repetitive.

I'd prefer open on both class and func to introduce only one keyword instead of two and possibly make it as a modifier of public to keep it in line with private(set)...

On Jul 6, 2016, at 1:11 AM, 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:

   swift-evolution/0117-non-public-subclassable-by-default.md at master · apple/swift-evolution · GitHub

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

   swift-evolution/process.md at master · apple/swift-evolution · GitHub

Thank you,

-Chris Lattner
Review Manager

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

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