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


(Chris Lattner) #1

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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


(Xiaodi Wu) #2

        * What is your evaluation of the proposal?

This is improved from the previous iteration. The code example needs
updating, as both instances of `open func bar()` should be `public open
func bar()` as outlined in the Proposed Design section.

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

Yes, mostly. There is one comment in the code example that describes a
restriction which does not fit with the direction of Swift. It is not the
main focus of the proposal but I think should be changed. Namely, the
proposal comments:

"[The declaration `[public] open func bar()` inside a class not marked
`open`] raises a compilation error: a method can't be marked `open` if the
class it belongs to can't be subclassed."

This is discordant with the direction resolved by the core team in the
SE-0025 revisions, where it was stated with regard to access modifiers:

"The compiler should not warn when a broader level of access control is
used within a type with more restrictive access, such as `internal` within
a `private` type. This allows the owner of the type to design the access
they would use were they to make the type more widely accessible."

Applying the same rationale here would suggest that the compiler should not
raise an error if a method is marked `open` inside a non-`open` type, in
order to allow the owner of the type to design as though to make it
subclassable without actually having to do so.

        * If you have used other languages or libraries with a similar

feature, how do you feel that this proposal compares to those?

Yes, I've used OOP in other languages. As discussed, this approach is
different from that taken by many of those but is a deliberate step.

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

Followed the discussion, read proposal carefully.

···

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) #3

  * What is your evaluation of the proposal?

+1 as before but I do have concerns

  * Why do open classes need to have also have open superclasses?
  * I don’t think sealed methods/properties as default is the right default.
  * If the default was open for methods/properties, then do we really need the sealed keyword? Could’t we just use final for that?

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

Requiring super classes to also be Open seems wrong.

I agree with Kevin Ballard on being open by default for methods/ properties http://article.gmane.org/gmane.comp.lang.swift.evolution/23467/

If we are open by default for methods/properties then we could simplify things by just using final for methods that need to be sealed; I don’t see the need to have sealed methods/properties outside of the module ONLY.

We already have to mark all public methods/properties as public, if I need something not the be overridable then I can just use final. This would work the same across all classes.

If I was a framework author that initially just had a library that was sealed but then somebody asked me to make it open; All I would want to do is add open to the class only, I would not want to spend the time to go back an add open to all public methods/properties specially if I am already using final. I think having method/properties open by default is the best compromised. I can see framework authors switching a class to be open and expecting that everything in the class in open and if they don’t open any methods, then what possible use is a subclass than an extension could not provide? I think that is an overreach to make the framework author have to add open to every public method in addition to having to open every super class. As a framework author if I think my clients could use subclassing, all I want to do is flip a switch on the classes that I want to make subclass able and then just push a new version. As a framework user I want to be able to tell my framework author “can you just add open to classes abs and deca etc.” I think it is more likely that I will get my request if it is easy vs it the framework author needs to touch every class in the hierarchy.

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

reviewed previously and followed the update.


(Taras Zakharko) #4

  * What is your evaluation of the proposal?

+1. Looks much better than the previous version and makes a lot of sense to me.

A quick clarification question (I didn’t find any mention of this in the proposal, but I might have missed it): what happens if a class is declared open, but does not contain any open member declarations? Is this legal?

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

Most likely

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

Yes

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

A quick reading. I was also following the previous discussion.

···

On 16 Jul 2016, at 07:52, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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?
  * 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


(Andre) #5

2016/07/16 14:52、Chris Lattner via swift-evolution <swift-evolution@swift.org> のメール:

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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 for the improved keywords and purgatory for objc imported modules.

This is much cleaner than the previous proposal, and cant wait to actually get to use this (as a framework author it literally makes me giddy).

Like others, I slightly wonder what will happen in The Real World™ (as a framework consumer) but if things end up too restrictive, it can always be changed down the road I would expect…?

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

This seems closer to Kotlin; not that I am much of a user of that language so maybe I am unqualified to say, but I think its nice to align with a good design that exists elsewhere…
Objc is of course the exact opposite of this, so it remains to be seen what the effect will be in this community...

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

Been following this intensely since last year’s initial discussion, finally came out of the shadows to comment when it came up for review, and have been weighing and listening to arguments for and against; my only "against" feelings came about as a consumer of APIs not as a writer, so I want to make sure its apparent that any negatives I had in the past were from that perspective; I very much support the spirit of this proposal and appreciate those that took time to explain and assuage those concerns! What an amazing process this is. ^o^

···

-----------

One thing in the proposal:
The superclass of an open class must be open. <snip/>. These are conservative restrictions that reduce the scope of this proposal; it will be possible to revisit them in a later proposal.
Yes, maybe its an anit-pattern, but I definitely have made many private parent classes and have the child public… the above would mean that I cant do that, as open needs public… it would definitely be appreciated if that is revisited or at least the reasoning (e.g we really shouldn't be making subclasses of private superclasses public because XYZABC etc).

-----------

Thanks!!

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


(Brent Royal-Gordon) #6

  * What is your evaluation of the proposal?

I agree with the core team that closed by default for classes is the right move.

I also think that it makes sense for methods to be closed by default. **However**, I am seriously concerned about the source-code impact of this change. I believe there are at least an order of magnitude more public member declarations than public class declarations in most projects. For instance, I attempted (using crude, most likely imperfect regex-based algorithms) to estimate the number of classes, methods, properties, and subscripts in Corelibs Foundation, and got these results:

  class: 120
  func: 1277
  subscript: 6
  var: 707

(Note: I removed obviously incorrect things like top-level constants and functions, but these numbers probably still count some struct and enum members.)

Annotating 120 types is not that big a deal relative to the size of Foundation. Annotating 1,990 members introduces much more clutter and tremendously increases the burden of closed-by-default. I don't think I can support burdening that many declarations with extra syntax; it seems like a lot of red tape for a case where you've already explicitly opted in to subclassing.

That's why I prefer the alternative design of having `open` as a separate access level. I don't think it is a serious burden to have to search for "public|open" when you want to see all public APIs. Nor do I think it's a problem that `open` is short. A short keyword would be a problem if our goal is to actually *discourage* subclassing, but if we merely want people to *think* before they subclass, we should be happy that permitting subclassing and overriding is not encumbered with heavyweight keywords.

Meanwhile, the benefits of an `open` access level are manifold:

* It ensures that unsealing is no more burdensome than sealing.

* There's no need for a "you can't declare it open because it's not public" diagnostic.

* It defuses one of the complaints about this change from its critics, easing acceptance.

As an alternative to having `open` as a separate access level, we could instead have it merely imply `public`: the canonical form would be `public open`, but source code could just say `open`. Generated interfaces would always say `public open`, so searching for `public` in those would work as you want it to.

So, in short:

1. Yes on closed classes by default.

2. No on closed members by default, unless we use a syntax less burdensome than `public open`.

3. I think the arguments against standalone `open` are weak, and I would strongly prefer it to `public open`.

  * 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've used OO languages, but not sealing ones.

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

Quick reading of this draft, but deep involvement in previous reviews and discussions.

···

On Jul 15, 2016, at 10:52 PM, Chris Lattner <clattner@apple.com> wrote:

--
Brent Royal-Gordon
Architechies


(L Mihalkovic) #7

Regards
(From mobile)

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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?

Part of a series of increasingly compelling arguments for switching other languages for writting ios/osx application, provided that that is not also prohibited in the various stores in the near future

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

No. I'll self censor the rest as it is not flatering

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

Feel: not surewhat the feel of swifft is supposed to be anymore
Direction: yes... training wheels all around, limited abilitiy to organize & structure code (other key features missing for that & plenty of real life examples on github to show this is actually the case)

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

The bulk my professional experience has been mostly with asm x86, c, c++, perl, java, scala, tcl/tk, go, xtend, vb, c#, fortran, cobol, javascript and recently TypeScript. None have something equivalent. I recently started toying with kotlin, that looks at inheritence in a similar light, but do not have enough real life experience yet to speak.
As for TypeScript, I only recently started writing large amounts of it professionally, and I am absolutely blown away: it has been the easiest language to learn and become extremely productive with, thanks to the most sound generic type system I have ever used (for bkgrnd, I love and makes very heavy use of the java/c# generics).

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

A lot.

···

On Jul 16, 2016, at 7:52 AM, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

More information about the Swift evolution process is available at

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

Thank you,

-Chris Lattner
Review Manager

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


(Tino) #8

I naturally assumed that "public" and "open" would be two separate concepts, as it has been expressed that orthogonality* is favored.
But actually reading the proposal, it says:
"open is invalid on declarations that are not also public", which imho not only is an unnecessary mingling of the two concepts, it also blocks the option to declare methods that can't be called outside the framework, which isn't that uncommon in Cocoa (methods like UIView.drawRect wouldn't show up in autocompletion lists anymore).

The whole proposal is about limitation whose rationale is incomprehensible for many, but for this "restriction of the restriction", I can't see any rationale at all.

- Tino

* At class-level, there is afaics no orthogonality planned as well (a class that is abstract outside its module… might be useful as well)


(Tino) #9

I naturally assumed that "public" and "open" would be two separate concepts, as it has been expressed that orthogonality* is favored.
But actually reading the proposal, it says:
"open is invalid on declarations that are not also public", which imho not only is an unnecessary mingling of the two concepts, it also blocks the option to declare methods that can't be called outside the framework, which isn't that uncommon in Cocoa (methods like UIView.drawRect wouldn't show up in autocompletion lists anymore).

The whole proposal is about limitation whose rationale is incomprehensible for many, but for this "restriction of the restriction", I can't see any rationale at all.

- Tino

* At class-level, there is afaics no orthogonality planned as well (a class that is abstract outside its module… might be useful as well)


(Karoy Lorentey) #10

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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, with notes:

1. The interaction of open with the dynamic keyword should be specified. Does "public dynamic" imply “open”? Dynamic provides a level of flexibility beyond mere subclassing, so I believe it should. Dynamic already conflicts with “final”, so there is precedent for this kind of interaction in the language. Note that “public dynamic” implying “open” means that we can’t have public dynamic members in a public class that’s not also open. I think this restriction is reasonable.

2. What about @objc methods? The docs say that it makes a name available but doesn’t guarantee dynamic dispatch on its own; so, it looks mostly irrelevant to this proposal.

3. The problem of code migration should be addressed. For example, we might want the auto-translator to automatically add open to non-final public methods/properties in existing code, to make it less painful to upgrade. People who simply accept the conversion results will get stuck with un-audited open stuff all over their public APIs, which is not ideal. On the other hand, this is no different to how their existing code behaved in Swift 2, so perhaps it is the best choice.

4. I don’t have a strong opinion on whether “open" should imply “public". If we accept that “public dynamic” is a stronger promise than “public open", then it would look strange that the former requires public, while the latter doesn’t. On the other hand, “public open” is going to be much more frequently used than “public dynamic”, so arguably it deserves special treatment.

5. I was suprised by the restriction that the superclass of open classes must also be open. But I don’t have a convincing usecase against it, so I guess it’s fine. I like that we have the option to revisit this later.

For fun, here are the distinct combinations of access levels and dispatch clauses for members in a "public open" class after this proposal:

public dynamic // plus “public dynamic open” if we keep it separate.
[public] open // assuming “open” implies “public"
public
public final
[internal] dynamic
[internal]
[internal] final
private dynamic
private
private final

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

Absolutely.

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

Yep.

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

“Don’t use public subclassing" has been my policy for years, but I have not had the pleasure to use a language that helps me enforce this.

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

In-depth study broken across many short sessions.

···

On 2016-07-16, at 07:52, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

--
Karoly
@lorentey


(Scott James Remnant) #11

I reviewed the original proposal, and this revised text is a much better step forwards; I still think there are a few changes I’d like to see before fully getting behind it.

Consider this a +0.5.

I like the use of the additional `open` keyword on top of `public`, it has a nice symmetry to the use of the additional `final` keyword for the opposite effect. I’m not totally sold on the actual name of the keyword, but I’m not going to argue over the color of that bikeshed.

That `open` is invalid on declarations that are `final` feels to me like these are keywords of the same family, just as `public` and `private` make no sense together. I like that.

I disagree that an `open` method overridden from a superclass is implicitly `open`.

As the rationale for the proposal states, overridability is hard to get right, and there is no guarantee that the consumer of an API is going to think about it. The default for override methods should not be `open` or `final`, it should be the internal equivalent.

A coder subclassing a public API, who themselves wants their subclass to be subclassable, should need to restate `open` where appropriate.

This also seems to be a general conflict in that you can always reduce the access, e.g. an API might have a `public open` method, but the subclass should be able to declare that as `override private` and thus the `open` keyword would be invalid in this context.

Scott


(Amber, SimpleTouch) #12

* What is your evaluation of the proposal?

Quoting Motivation : "The major observation here is that not all classes
make sense to subclass, and it takes real thought and design work to make a
class subclassable *well*. As such, being able to subclass a public class
should be an additional "promise" beyond the class just being
marked public."

There seems to be two questions.

   1. should a class being designed be sealed or not? ’final’ already
   works, changing defaults seems unnecessary.
   2. how does the class designer go about selecting what to expose for
   possible overriding in sub-classes. I agree with the proposal.
   Selectively publishing members is a better way to think about the class
   design, and will result in clearer intent and better design.

Suggestions:

   1. leave `final` as is
   2. require explicit annotation of members for overriding. ( `default`,
   `published`, `open` )
   3. whether a class or member is public, internal, or private should have
   no bearing on number 1 or 2.
   4. if there are no `published` members, class is implicitly final.

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

As proposed the ideas are very much in line with Swift, hopefully as
suggested even more so.

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

A moderate amount of thought and experimentation. I did not have time to
read historical discussions.

Amber

···

On Sat, Jul 16, 2016 at 1:52 AM Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable
publicly" begins now and runs through July 22. 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


(Juan Laube) #13

* What is your evaluation of the proposal?

-1, I really think this a step in the wrong direction.
I recognise the problem around this, and why something is needed. However, I don’t like the idea of restricting things by default. In the attempt to solve a problem, we will create more problems by introducing more workarounds to replace what now is being overridden.

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

No. I think the defaults should not be changed, but restricting subclassing (and overriding) should be opt-in.

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

No. I think this kind of restrictions make Swift worst.

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

This is the opposite of how other languages solves this kind of restrictions.

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

I read in-depth the updated proposal and most of the discussion.

···

On Jul 16, 2016, at 2:52 AM, Chris Lattner <clattner@apple.com> wrote:

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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


(Gwynne Raskind) #14

  * What is your evaluation of the proposal?

With all respect to those who have put a lot of work into this proposal, I’m reluctantly and immensely against it.

The reasoning from the core team that working with vendors replaces the ability to subclass to work around problems simply doesn’t hold water for me. This is a very real and very common issue, and there are endless cases where vendors won’t or even *can’t* solve the problem, especially in closed-source code. And it’s not just a matter of GUI applications; I’ve run into the need to subclass (especially in order to deliver working results in reasonable time) in projects as low-level and open-source as LLVM.

In short, saying "filing a bug will work" isn’t good enough to justify locking out the ability of developers to deal with problems in vendor code. It’s simply not true - it’s certainly almost never been true of Apple frameworks, and even when it has, that doesn’t help anyone "now". (To be clear, I’m not suggesting Apple is unresponsive to Radars. However, I am saying that there’s no transparency, no confidence in getting fixes, and no hope of any kind of reasonable (from a local perspective) timeline for deployment of fixes.)

In a perfect world, I agree this would be the case and *then* this proposal would be a fantastic concept that I’d be completely behind. Unfortunately, this is not a perfect world and the gains in encouraging good design and cleanliness of applying LSP (among other things) do not make up for the burden of implementation time and cost on framework users.

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

Yes. But the change is too extreme and the ugly truth of the real world makes this proposal an inferior solution.

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

Absolutely. If not for the practical considerations, I’d love it! Conceptually speaking, I find it elegant, even harmonious. But again, in practice, the result isn’t so pretty.

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

Denial of subclassing has always been opt-in in ever other language I’ve used (C++ and Java, to name two, and Objective-C (and older C++) don’t even have that much). Sealing a class against subclassing is one thing, but not providing any kind of escape hatch, any kind of IUnderstandThatSubclassingMayCauseSunsToGoNovaOrGalaxiesToExplode marker, hamstrings all users of the code. Opt-in sealing at least constrains this scenario to places where the framework writer thought it was worth adding the extra protection against horrible horribleness.

For example, it makes sense to seal NSSomeCryptographicInterface because you’re making it that much harder for something to deliberately interfere with or accidentally break crypto. It makes less sense to seal NSSplitViewController - I’d much rather Apple just said "tough noogies, your subclass breaks in this version because you did something unsupported" when fixing bugs!

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

I’ve been watching the discussion on this topic back and forth for some time now, and I read both revisions of the proposal carefully. I’m very much conceptually in favor of it, and I regret that I can’t stand behind it! But the fact remains that I think it’s a bad idea.

-- Gwynne Raskind

···

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


(David Waite) #15

  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 don’t necessarily like the idea of sealed-by-default overridability of members, although I will admit it goes along well with the general philosophy of reducing the need for modifiers on internal API. I would prefer final-by-default, and the final keyword being removed.

I did not see anything within the proposal, but I assume that overridability of initializers was evaluated and deemed sufficient with existing mechanisms?

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

Generally yes

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

Sure, I would say C# follows this reasonably well with its virtual keyword, although I don’t believe they are sealed by default at the class level

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

Followed most of comments, In-depth study

-DW


(ilya) #16

Hello to all of the community.

* * What is your evaluation of the proposal?*
+0.5
Agree on the motivation and 'public open class'
Let's discuss 'public open func' + application to dynamic runtime

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

My perspective is as follows.

We are designing a language that has a concept of classes and overrides.
Let's create a class and put some functions into it with the default
modifiers.

*Should it be possible to override those functions in the other parts of
the code?*

This is a deep question of language design. An answer will have profound
ramifications regarding the code that the compiler is allowed to produce.
It's not surprising that different languages give different answers:

- Java and Objective-C say "Yes"
- C++ and C# say "No"

Now, it doesn't look to me like it's "impossible to use libraries" in those
last two languages or that one has "less fun", or that they are
"disadvantaged on the server" because of their design choices.

What it looks like is a tradeoff. A freedom in one place is a requirement
in another place – if you say "one should be able to override those
methods", it means "a compiler should be required to perform virtual
dispatch on any call to those methods".

A smart compiler will be able to perform some guaranteed optimizations if
it knows that the calls can be devirtualized. This is a good thing, because
you can perfrom refactoring "for free".

As an example, in the scenario below Pyramid.computeVolume() can be
devirtualized even if you refactor this class as a subclass of an abstract
"Geometric-Body" class and make computeBase() overrideable.

    class Pyramid {
         func computeVolume() { return computeBase() * height / 3}
         func computeBase() { return π * radius * radius }
    }

This ability to extract away the code without the performance penalty and
without turning it into an unwanted override point is a benefit of the "No"
answer to the aforementioned question.

The ability to quickly design a hierarchy of classes without worrying first
about making them virtual is a benefit of the "Yes" answer to the
aforementioned question.

What would a good tradeoff then look like? It would combine the advantages
of both approaches. I believe that a default "Yes" answer within a module
and "No" across the boundary does that.

Therefore I feel like this proposal moves Swift in the right direction.

However, I'm not sure that 'open' is the right keyword for functions and
would prefer to see more discussion regarding it.

It's less clear what the benefits of non-open classes are in the dynamic
runtime, as we can't really promise that they will not be subclassed.
Incidentally, a lot of the criticism of the proposal touches the usage in
Swift of Cocoa Touch APIs. Maybe there is value in leaving things exactly
as they currently are in the Objective-C.

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

This is the first proposal I feel compelled to respond to.

Many interesting points regarding the proposal have been raised. I've spent
some time trying to read most of the thread and thought about this question
during a day. I also refreshed my memory regarding other languages, and the
design patterns of inheritance in general, e.g. here:

- https://codeblog.jonskeet.uk/2006/03/04/inheritancetax/

···

-
http://blog.berniesumption.com/software/inheritance-is-evil-and-must-be-destroyed/
- http://www.artima.com/intv/nonvirtualP.html
- http://stackoverflow.com/a/14451437/115200 (Eric Lippert)

Thank you if you got up to here,
Ilya.

On Sat, Jul 16, 2016 at 7:52 AM, Chris Lattner via swift-evolution < swift-evolution@swift.org> wrote:

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable
publicly" begins now and runs through July 22. 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


(John McCall) #17

  * What is your evaluation of the proposal?

+1. Looks much better than the previous version and makes a lot of sense to me.

A quick clarification question (I didn’t find any mention of this in the proposal, but I might have missed it): what happens if a class is declared open, but does not contain any open member declarations? Is this legal?

Yes. Subclasses can extend the API of the class and add new properties, but they can't modify the core behavior of the superclass. That's not an implausible class design, especially as a first step, since it would still be source/binary-compatible to make a method open in a later release.

John.

···

On Jul 16, 2016, at 12:33 AM, Taras Zakharko via swift-evolution <swift-evolution@swift.org> wrote:

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

Most likely

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

Yes

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

A quick reading. I was also following the previous discussion.

On 16 Jul 2016, at 07:52, Chris Lattner via swift-evolution <swift-evolution@swift.org> wrote:

Hello Swift community,

The second review of "SE-0117: Default classes to be non-subclassable publicly" begins now and runs through July 22. 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?
  * 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

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


(John McCall) #18

  * What is your evaluation of the proposal?

+1 as before but I do have concerns

  * Why do open classes need to have also have open superclasses?

I talk about this in the proposal; this is a possible avenue of future extension. I wanted to keep this proposal small, and requiring the whole hierarchy to be open avoids a number of design problems.

  * I don’t think sealed methods/properties as default is the right default.
  * If the default was open for methods/properties, then do we really need the sealed keyword? Could’t we just use final for that?

There is no "sealed" keyword in the proposal.

Since we're naming open "open", if we need a reverse, we would probably use "closed" or "nonopen" for it. "sealed" is a common term for this kind of feature, and it's the one we've been using in this discussion, but unfortunately "sealed" is not a natural opposite of "open".

John.

···

On Jul 16, 2016, at 9:05 AM, Jose Cheyo Jimenez via swift-evolution <swift-evolution@swift.org> wrote:

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

Requiring super classes to also be Open seems wrong.

I agree with Kevin Ballard on being open by default for methods/ properties http://article.gmane.org/gmane.comp.lang.swift.evolution/23467/

If we are open by default for methods/properties then we could simplify things by just using final for methods that need to be sealed; I don’t see the need to have sealed methods/properties outside of the module ONLY.

We already have to mark all public methods/properties as public, if I need something not the be overridable then I can just use final. This would work the same across all classes.

If I was a framework author that initially just had a library that was sealed but then somebody asked me to make it open; All I would want to do is add open to the class only, I would not want to spend the time to go back an add open to all public methods/properties specially if I am already using final. I think having method/properties open by default is the best compromised. I can see framework authors switching a class to be open and expecting that everything in the class in open and if they don’t open any methods, then what possible use is a subclass than an extension could not provide? I think that is an overreach to make the framework author have to add open to every public method in addition to having to open every super class. As a framework author if I think my clients could use subclassing, all I want to do is flip a switch on the classes that I want to make subclass able and then just push a new version. As a framework user I want to be able to tell my framework author “can you just add open to classes abs and deca etc.” I think it is more likely that I will get my request if it is easy vs it the framework author needs to touch every class in the hierarchy.

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

reviewed previously and followed the update.

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


(John McCall) #19

        * What is your evaluation of the proposal?

This is improved from the previous iteration. The code example needs updating, as both instances of `open func bar()` should be `public open func bar()` as outlined in the Proposed Design section.

Good catch. I'll fix this.

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

Yes, mostly. There is one comment in the code example that describes a restriction which does not fit with the direction of Swift. It is not the main focus of the proposal but I think should be changed. Namely, the proposal comments:

"[The declaration `[public] open func bar()` inside a class not marked `open`] raises a compilation error: a method can't be marked `open` if the class it belongs to can't be subclassed."

This is discordant with the direction resolved by the core team in the SE-0025 revisions, where it was stated with regard to access modifiers:

"The compiler should not warn when a broader level of access control is used within a type with more restrictive access, such as `internal` within a `private` type. This allows the owner of the type to design the access they would use were they to make the type more widely accessible."

Applying the same rationale here would suggest that the compiler should not raise an error if a method is marked `open` inside a non-`open` type, in order to allow the owner of the type to design as though to make it subclassable without actually having to do so.

That's true. We'll consider this.

John.

···

On Jul 16, 2016, at 12:59 AM, Xiaodi Wu via swift-evolution <swift-evolution@swift.org> wrote:

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

Yes, I've used OOP in other languages. As discussed, this approach is different from that taken by many of those but is a deliberate step.

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

Followed the discussion, read proposal carefully.

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


(Jean-Daniel) #20

  * What is your evaluation of the proposal?

+1 as before but I do have concerns

  * Why do open classes need to have also have open superclasses?
  * I don’t think sealed methods/properties as default is the right default.
  * If the default was open for methods/properties, then do we really need the sealed keyword? Could’t we just use final for that?

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

Requiring super classes to also be Open seems wrong.

I agree with Kevin Ballard on being open by default for methods/ properties http://article.gmane.org/gmane.comp.lang.swift.evolution/23467/

If we are open by default for methods/properties then we could simplify things by just using final for methods that need to be sealed; I don’t see the need to have sealed methods/properties outside of the module ONLY.

We already have to mark all public methods/properties as public, if I need something not the be overridable then I can just use final. This would work the same across all classes.

If I was a framework author that initially just had a library that was sealed but then somebody asked me to make it open; All I would want to do is add open to the class only, I would not want to spend the time to go back an add open to all public methods/properties specially if I am already using final. I think having method/properties open by default is the best compromised. I can see framework authors switching a class to be open and expecting that everything in the class in open and if they don’t open any methods, then what possible use is a subclass than an extension could not provide?

Extensions do not support stored properties (yet?). Subclasses do. That said, I agree that having an open class without any open member does not has much benefit, but I’m sure we can find a valid use case for such class.

···

Le 16 juil. 2016 à 18:05, Jose Cheyo Jimenez via swift-evolution <swift-evolution@swift.org> a écrit :

I think that is an overreach to make the framework author have to add open to every public method in addition to having to open every super class. As a framework author if I think my clients could use subclassing, all I want to do is flip a switch on the classes that I want to make subclass able and then just push a new version. As a framework user I want to be able to tell my framework author “can you just add open to classes abs and deca etc.” I think it is more likely that I will get my request if it is easy vs it the framework author needs to touch every class in the hierarchy.

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

reviewed previously and followed the update.

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