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

2016/07/09 23:30、Matthew Johnson <matthew@anandabits.com <mailto:matthew@anandabits.com>> のメール:

This leaves the scenario of a case where you depend on a 3rd party, closed-source library written in Swift and where you cannot get (or use) a fix from the vendor for some reason. This is a legitimate concern, but IMO it is not large enough to outweigh all of the advantages of making sealed the default.

What are your thoughts on an ability for a way to force unseal a class that does need to be fixed, even if its temporary?

Something like:

class MyFixedClass : @forceUnseal(SomeSealedClassThatNeedsFixing) { //Emits a scary compiler warning
}

Does that even seem feasible/possible, much less reasonable…?
Though it would have to be a perhaps separate discussion, this comes to my mind as becoming necessary down the road, but maybe I’m wrong...

I'm not opposed to something like this in principle, but I'm not sure how it would work in practice. There was some discussion of something along these lines on the list at one point (I think Joe Groff had some ideas). However, I don't think this is possible if the optimizer takes advantage of the sealed status when the library is compiled. I'll leave it to the compiler experts to comment further on feasibility.

My technical analysis: It’s certainly implementable to have the optimizer not take advantage of the sealed status, and to allow some sort of “unsafe-break-the-seal” syntax. We’d have to be sure that anything that “can’t possibly happen without external subclassing” still at least generates a deterministic trap rather than memory corruption, but that’s probably doable.

(I’ll leave it at that, without trying to argue a particular side.)

I have seen some comments about nontrivial complexity in Apple’s frameworks caused by apps subclassing where they should not have (i.e. classes that would be sealed if it were possible in Objective-C). This is extremely unfortunate and it impacts everyone on Apple’s platforms.

I wish I had links handy for you, but I don’t recall exactly where or when this was mentioned and don’t have time to dig them up right now.

I see, thats reasonable… if those links are available somewhere I would definitely like to see them, it would be a good education for me…

IIRC like Jordan Rose may have made some comments along these lines either on list or on Twitter if you want to search, but that is a fuzzy memory and could easily be wrong. :)

I don’t have anything handy (partially because some of it isn’t public knowledge), but it’s a well-known phenomenon within Apple that new OSs break third-party apps in strange ways because they are relying on being able to swizzle a non-public selector, or even on its existence. I’ll admit I don’t hear as much about intrusive subclassing, but that doesn’t mean Apple hasn’t made changes that assume no one subclasses a particular class.

[For anyone who doesn’t know the term “method swizzling”: Objective-C allows you to replace a class’s implementation of a method at run-time, regardless of where the class or the replacement is defined.]

Jordan

Right, sorry, I mis-spoke. The intent of @testable is to allow tests to have special privileges as if they were part of the same module, but of course it doesn't actually make them part of the same module, and there any number of lookup / redeclaration differences.

Still, we're agreed on how that principle applies here: tests should be able to subclass / override things arbitrarily from the things they @testably import.

John.

···

On Jul 8, 2016, at 8:41 PM, Jordan Rose <jordan_rose@apple.com> wrote:

On Jul 6, 2016, at 09:16, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

On Jul 5, 2016, at 10:56 PM, Chris Lattner <clattner@apple.com <mailto:clattner@apple.com>> wrote:

On Jul 5, 2016, at 6:53 PM, John McCall via swift-evolution <swift-evolution@swift.org <mailto:swift-evolution@swift.org>> wrote:

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

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

That makes sense to me. Please explicitly add that to the proposal, thank you!

Done.

This really isn’t the model for @testable, as evidenced by the fact that top-level names in the testing module still shadow names from the imported module, and that you can refer to the name fully-qualified. Instead, the model is that @testable makes ‘internal' things ‘public'. I think this would make them ‘subclassable’/‘overridable’/‘open’ instead where relevant.

Reposting Károl reply on this list.

···

On Sat, Jul 9, 2016 at 11:01 PM, Károly Lőrentey <karoly@lorentey.hu> wrote:

> On 2016. Jul 9., at 22:55, Goffredo Marocchi <panajev@gmail.com> wrote:
>
> Why have they not "fixed" this issue with Java 6/7/8 if it is bad to
have the current setup by default? Why C++ x09/x11/x14 is also not making
everything sealed/unsubclassable by default?

I'd wager a guess that the strong desire not to break source compatibility
with existing code explains why Java and C++ are stuck forever with
suboptimal defaults. Some members of this list have a bit of background in
C++ language design (understatement of the day!); perhaps they know more.

> Is it possible that having library authors use something like a sealed
keyword or similar is good enough for the default case?

Swift is to be safe by default. I believe open subclassability is a power
tool that's unsafe without adequate training and thick protective gear;
therefore, it is useful to require posting yellow/black hazard signs when
it is in use. Safety first.

"Opting for safety sometimes means Swift will feel strict, but we believe
that clarity saves time in the long run."

Karoly
@lorentey

Quick P.S.: I just remembered that JetBrains' Kotlin exists, and it made final classes the default:

"By default, all classes in Kotlin are final, which corresponds to Effective Java, Item 17: ‘Design and document for inheritance or else prohibit it’."
  -- https://kotlinlang.org/docs/reference/classes.html#inheritance

They even quote the same authority as I did! <3

It seems to have worked out fine in practice, except for some bad interactions with some standard Java packages -- which I do not expect to be an issue in our case.

···

On 2016-07-09 22:01:38 +0000, Károly Lőrentey via swift-evolution said:

On 2016. Jul 9., at 22:55, Goffredo Marocchi <panajev@gmail.com> wrote:

Why have they not "fixed" this issue with Java 6/7/8 if it is bad to have the current setup by default? Why C++ x09/x11/x14 is also not making everything sealed/unsubclassable by default?

I'd wager a guess that the strong desire not to break source compatibility with existing code explains why Java and C++ are stuck forever with suboptimal defaults. Some members of this list have a bit of background in C++ language design (understatement of the day!); perhaps they know more.

--
Károly
@lorentey

Regards
(From mobile)

Why have they not "fixed" this issue with Java 6/7/8 if it is bad to have the current setup by default? Why C++ x09/x11/x14 is also not making everything sealed/unsubclassable by default?

I'd wager a guess that the strong desire not to break source compatibility with existing code explains why Java and C++ are stuck forever with suboptimal defaults.

May I suggest that you might want to read about it if you have any interest in making accurate statements about it.

Some members of this list have a bit of background in C++ language design (understatement of the day!); perhaps they know more.

Is it possible that having library authors use something like a sealed keyword or similar is good enough for the default case?

Swift is to be safe by default. I believe open subclassability is a power tool that's unsafe without adequate training and thick protective gear; therefore, it is useful to require posting yellow/black hazard signs when it is in use. Safety first.

"Opting for safety sometimes means Swift will feel strict, but we believe that clarity saves time in the long run."

It each their own... some people like the mental comfort of others building safeguard for them, others like to live baring the full weight of the consequences of their choices. Nothing wrong, just two different approaches to life.

···

On Jul 10, 2016, at 12:01 AM, Károly Lőrentey via swift-evolution <swift-evolution@swift.org> wrote:

On 2016. Jul 9., at 22:55, Goffredo Marocchi <panajev@gmail.com> wrote:

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

2016/07/09 23:30、Matthew Johnson <matthew@anandabits.com <mailto:matthew@anandabits.com>> のメール:

This leaves the scenario of a case where you depend on a 3rd party, closed-source library written in Swift and where you cannot get (or use) a fix from the vendor for some reason. This is a legitimate concern, but IMO it is not large enough to outweigh all of the advantages of making sealed the default.

What are your thoughts on an ability for a way to force unseal a class that does need to be fixed, even if its temporary?

Something like:

class MyFixedClass : @forceUnseal(SomeSealedClassThatNeedsFixing) { //Emits a scary compiler warning
}

Does that even seem feasible/possible, much less reasonable…?
Though it would have to be a perhaps separate discussion, this comes to my mind as becoming necessary down the road, but maybe I’m wrong...

I'm not opposed to something like this in principle, but I'm not sure how it would work in practice. There was some discussion of something along these lines on the list at one point (I think Joe Groff had some ideas). However, I don't think this is possible if the optimizer takes advantage of the sealed status when the library is compiled. I'll leave it to the compiler experts to comment further on feasibility.

My technical analysis: It’s certainly implementable to have the optimizer not take advantage of the sealed status, and to allow some sort of “unsafe-break-the-seal” syntax. We’d have to be sure that anything that “can’t possibly happen without external subclassing” still at least generates a deterministic trap rather than memory corruption, but that’s probably doable.

(I’ll leave it at that, without trying to argue a particular side.)

I agree that this is implementable. I would be very reluctant to do it, though; it amounts to writing off all of the performance advantages of the proposal. (And possibly the semantic advantages as well, like being less restrictive about required initializers.)

John.

···

On Jul 10, 2016, at 6:42 PM, Jordan Rose via swift-evolution <swift-evolution@swift.org> wrote:

I have seen some comments about nontrivial complexity in Apple’s frameworks caused by apps subclassing where they should not have (i.e. classes that would be sealed if it were possible in Objective-C). This is extremely unfortunate and it impacts everyone on Apple’s platforms.

I wish I had links handy for you, but I don’t recall exactly where or when this was mentioned and don’t have time to dig them up right now.

I see, thats reasonable… if those links are available somewhere I would definitely like to see them, it would be a good education for me…

IIRC like Jordan Rose may have made some comments along these lines either on list or on Twitter if you want to search, but that is a fuzzy memory and could easily be wrong. :)

I don’t have anything handy (partially because some of it isn’t public knowledge), but it’s a well-known phenomenon within Apple that new OSs break third-party apps in strange ways because they are relying on being able to swizzle a non-public selector, or even on its existence. I’ll admit I don’t hear as much about intrusive subclassing, but that doesn’t mean Apple hasn’t made changes that assume no one subclasses a particular class.

[For anyone who doesn’t know the term “method swizzling”: Objective-C allows you to replace a class’s implementation of a method at run-time, regardless of where the class or the replacement is defined.]

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

So either open by default and sealed optionally or sealed by default and no
escape hatch?

···

On Mon, Jul 11, 2016 at 6:20 PM, John McCall via swift-evolution < swift-evolution@swift.org> wrote:

On Jul 10, 2016, at 6:42 PM, Jordan Rose via swift-evolution < > swift-evolution@swift.org> wrote:

2016/07/09 23:30、Matthew Johnson <matthew@anandabits.com> のメール:

This leaves the scenario of a case where you depend on a 3rd party,
closed-source library written in Swift and where you cannot get (or use) a
fix from the vendor for some reason. This is a legitimate concern, but IMO
it is not large enough to outweigh all of the advantages of making sealed
the default.

What are your thoughts on an ability for a way to force unseal a class
that does need to be fixed, even if its temporary?

Something like:

class MyFixedClass : @forceUnseal(SomeSealedClassThatNeedsFixing) {
//Emits a scary compiler warning
}

Does that even seem feasible/possible, much less reasonable…?
Though it would have to be a perhaps separate discussion, this comes to my
mind as becoming necessary down the road, but maybe I’m wrong...

I'm not opposed to something like this in principle, but I'm not sure how
it would work in practice. There was some discussion of something along
these lines on the list at one point (I think Joe Groff had some ideas).
However, I don't think this is possible if the optimizer takes advantage of
the sealed status when the library is compiled. I'll leave it to the
compiler experts to comment further on feasibility.

My technical analysis: It’s certainly implementable to have the optimizer
*not* take advantage of the sealed status, and to allow some sort of
“unsafe-break-the-seal” syntax. We’d have to be sure that anything that
“can’t possibly happen without external subclassing” still at least
generates a deterministic trap rather than memory corruption, but that’s
probably doable.

(I’ll leave it at that, without trying to argue a particular side.)

I agree that this is implementable. I would be very reluctant to do it,
though; it amounts to writing off all of the performance advantages of the
proposal. (And possibly the semantic advantages as well, like being less
restrictive about required initializers.)

John.

I have seen some comments about nontrivial complexity in Apple’s
frameworks caused by apps subclassing where they should not have (i.e.
classes that would be sealed if it were possible in Objective-C). This is
extremely unfortunate and it impacts everyone on Apple’s platforms.

I wish I had links handy for you, but I don’t recall exactly where or when
this was mentioned and don’t have time to dig them up right now.

I see, thats reasonable… if those links are available somewhere I would
definitely like to see them, it would be a good education for me…

IIRC like Jordan Rose may have made some comments along these lines either
on list or on Twitter if you want to search, but that is a fuzzy memory and
could easily be wrong. :)

I don’t have anything handy (partially because some of it isn’t public
knowledge), but it’s a well-known phenomenon within Apple that new OSs
break third-party apps in strange ways because they are relying on being
able to swizzle a non-public selector, or even on its existence. I’ll admit
I don’t hear as much about intrusive subclassing, but that doesn’t mean
Apple hasn’t made changes that assume no one subclasses a particular class.

[For anyone who doesn’t know the term “method swizzling”: Objective-C
allows you to replace a class’s implementation of a method at run-time,
regardless of where the class or the replacement is defined.]

Jordan
_______________________________________________
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

I think there are two separate decisions in here:

- Open by default or sealed by default?
- If sealed, can the seal be broken by clients?

These decisions aren’t independent ("if sealed is the default, it’s more important for clients to be able to break it, since it might be an accident”, perhaps), but either can be implemented independently.

Jordan

···

On Jul 11, 2016, at 11:16, Goffredo Marocchi <panajev@gmail.com> wrote:

So either open by default and sealed optionally or sealed by default and no escape hatch?