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


(Chris Lattner) #1

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


(Austin Zheng) #2

I'll bite.

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

···

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

        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


(Nikita Leonov) #3

+1 with modifications. In my team we write “final class” a lot. We also do have a lot of internal frameworks and an ability to guide external frameworks uses is important for us. We have a lot of candidates in our frameworks that should be inheritable internally, but we do not recommend to extend them outside of framework.

My biggest worry with this proposal is already highlighted in a section "Modifier spelling alternatives”. Proposal introduces a set of new keywords to express relatively similar concept as current `final` keyword. We currently have already an ability to limit inheritance by use `final`, but we can not define a scope. However we also have keywords to define an accessibility scope:
* public — accessible everywhere
* internal — accessible within a module
* private — accessible within a file

For example, by leveraging existing keywords we can write code like following:
final(internal) class Foo {
  final(private) func bar() {
  }
}
So the class `Foo` is inheritable within a module, but method bar is overridable only within same file.
There is also optional `final(public)` that literally means no limitations on inheritance that can be used to indicate that class or method is `final(public)` on purpose by design. We use the similar approach in our code base with `internal`, while it is default keyword for classes and functions we still write it to show that it is internal by design and no public / private keyword missing.

Also with suggestion above we can default `final` to `final(private)`to keep backwards compatibility.

Best,
Nikita Leonov

···

On Jul 5, 2016, at 4:11 PM, Chris Lattner <clattner@apple.com> 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-announce mailing list
swift-evolution-announce@swift.org
https://lists.swift.org/mailman/listinfo/swift-evolution-announce


(Brent Royal-Gordon) #4

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

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.

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

  * 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 think I have.

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

Quick reading of the final proposal, but I also contributed to previous discussion.

···

--
Brent Royal-Gordon
Architechies


(Kevin Lundberg) #5

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

Another point I can think of is mocking in tests. I'm unaware of any
fully featured mocking solution in swift, and subclassing a class to
override methods for mocking purposes is one way to make unit tests work
with third party dependencies. If a class was not designed for
subclassing, then writing tests where the class's behavior needs to be
suppressed would not be possible. One could write a facade between the
system under test and the dependency, but if the dependency is only used
in a one or a few places then that becomes a lot of extra boilerplate
code to write, especially if that process needed to happen many times.

I understand the value of this behavior though, and I would prefer that
this behavior becomes opt-in instead of opt-out. One possible
illustration (not meant as definitive syntax):

public(final) class Thing {}
public(final) func doSomething() {}

where the symbols are publicly final, but lesser access scopes are open.

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

Some change here would be welcome, but as I said I think the inverse
situation is a better solution.

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

It partially does, as giving developers tools to protect things they know are critical or sensitive is a good idea, but it should not come at the cost of a client's freedom to use a library in ways that fall outside of the original predictions of the author.

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

The main other language I've used is C# that does this at the method
level (where overridable methods must be marked with the `virtual`
keyword).

I've had issues before when working with third party C# code where the
author didn't declare a method as virtual, but the problem I was trying
to solve required me to override that method. Luckily the code was open
source, so I just embedded and edited the source into my project instead
of using the prebuilt binaries, but that becomes a maintenance headache,
and it is not possible for components that do not make the source available.

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

In depth study, and some participation on some of the initial threads
about this topic.


(Karl) #6

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. I really enjoy designing elegant, well-defined APIs and this behaviour for classes has bothered me for some time.

On the other hand, I have also subclassed things that weren’t meant to be subclassed (e.g. I believe UITabBar was an awkward one back in the day, and UINavigationController has so many broken behaviours it’s almost a requirement to subclass and patch it, even though I believe Apple disapproves). I could fall back to Objective-C, but that’s an implementation detail. In some theoretical future with an all-Swift UIKit, what do I do about those issues?

After thinking about it, I decided that in this theoretical future, no App would be able to use those hacks, and so all Apps would have the same broken behaviours and degrade the quality of the platform to such an extent that Apple is forced to do something about it. Ultimately, that’s exactly what we want; they need to see broken Apps everywhere in order to prioritise fixes. That improves code quality all-around.

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

Yup

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

Yup, but I don’t like “subclassable class”. My preferences are:

#1 - “extendable class” // explicitly clear about what I’m allowing
#2 - “open class” // this is a bit vague - open how? How is it different from public?

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

I prefer this over “sealed”. Making classes extendable requires careful consideration, and should be explicit.

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

I read through it a few times, and also the discussion last week. The problem has bugged me for a long time, and this is a pretty straightforward solution that integrates well with Swift (providing the keyword changes =] )

More information about the Swift evolution process is available at

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

Thank you,

My pleasure!

Karl

···

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


(Charlie Monroe) #7

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:

  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


(Chéyo Jiménez) #8

   * What is your evaluation of the proposal?

Plus one. Open is my keyword of choice.

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

Yes. But I think some sort of patch ability should be introduced as a compromised.
http://article.gmane.org/gmane.comp.lang.swift.evolution/1805/match=patch+sealed

Perhaps a way to have an extension on a sealed class to be marked as a patch that is able to hook up to the willSet/didSet on a sealed property or be able to run before or after a method function. I think something like this could ease the pain of a completely closed to patching system.

   * 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?
   * How much effort did you put into your review? A glance, a quick reading, or an in-depth study?

Moderate effort.

···

(plx) #9

+1 except for the choice of names if I understand it correctly, but I want to make sure I understand it correctly.

The scenario I want to to make sure I understand is what happens for a class-cluster type pattern like the following:

  // Module A:
  public class PublicBaseClass {
    func someMethod() { /* placeholder here */ }
  }

  private class PrivateSubclass : PublicBaseClass {
    override func someMethod() { /* new logic here */ }
  }

…I think this would make `PublicBaseClass` effectively “final” (e.g. non-subclassable) from outside of Module A, but `PrivateSubclass` (and similar constructs, etc.) would still be allowed and work as one would expect?

If so, that’s what I’d want semantically, so +1 on that for the semantics.

*But*, that behavior means that it is *very* confusing to be using `subclassable` and `overridable` for what they mean in this protocol.

I usually stay out of bike shedding but these attributes are really poorly named IMHO; I’d *highly recommend* something like either the ugly-but-explicit `externally_subclassable` / `externally_overrideable` or something else entirely (like maybe `open`?) that avoids the confusion.

Apologies if this has already come up or is based upon a misunderstanding of the proposal.

···

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


(Scott James Remnant) #10

-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


(Leonardo Pessoa) #11

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


(Jacopo Andrea Giola) #12

  * 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


(Sean Heber) #13

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


(Paul Cantrell) #14

* 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


(Tino) #15

-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


(Vladimir) #16

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


(Ricardo Parada) #17

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


(Michael Peternell) #18

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


(David Rönnqvist) #19

  * 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


(David Beck) #20

* 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