> On Jul 22, 2016, at 9:09 AM, Károly Lőrentey via swift-evolution
> <[email protected]> wrote:
>
> On 2016-07-21 15:33:37 +0000, Chris Lattner via swift-evolution said:
>> * What is your evaluation of the proposal?
>
> I gave enthusiastic thumbs up to the previous two proposals. I agree
> wholeheartedly with the underlying goal, and I love the new Motivation
> section. I'm OK with the idea of making "open" imply "public" by
> default, which seems to be the gist of the new design.
>
> However, reading the actual details in this third revision made me
> uncomfortable. I expected the specification to be bulletproof by now;
> but the actual text of the "Proposed design" section seems to be even
> less coherent than the previous two.
>
> I understand there's a (self-imposed) deadline, but this one could've
> really used a little more time in the oven.
>
> While I am still strongly in favor of the direction and high-level
> design of the proposal, I cannot in good conscience argue in favor
> of such ill-defined details in a _third_review_. So I'm giving
> this specific revision -1 in protest. 😢
I'm sorry if my drafting wasn't up to snuff. In my defense, I needed to get it
out quickly,
and I had a lot to rewrite. I was pretty tired by the time I actually finished
the motivation
section.
> Here are some of my problems with the text:
>
> 1. The proposal offers two separate design approaches for `open` classes,
> listing
> arguments for both. Please excuse my bluntness, but this is now the third
> attempt
> to push this through: With all due respect, would you please make up your
> mind?
Most of the proposal has been "pushed through", frankly. We are looking for
feedback
on this specific question. I agree that the proposal framework is an awkward
fit for
this kind of discussion prompt.
> 2. The option to get rid of sealed classes is brand new with SE-0117rev3.
> Did someone argue for it in review #2?
>
> (I'm all for adding better support for sealed class hierarchies, so I prefer
> plan A.)
>
> It's nice that we're given the option of reducing language complexity, but
> then
> it's bizarre that the idea of eliminating sealed _members_ isn't mentioned
> at all.
Because it was settled in the core-team review.
> Although, a literal reading of "plan B" does in fact seem to imply that any
> member
> that is not explicitly marked open would not even be _internally_
> overridable:
>
> "The second design says that there is no such thing as an open class
> because
> all classes are subclassable unless made final. Note that its direct
> methods
> would still not be overridable unless individually made open, although its
> inherited open methods could still be overridden."
>
> There is no mention of "public", and no mention of module boundaries.
> Thus, plan B basically proposes to make *all members* in *all classes*
> `final` by default.
> This seems inconsistent with the rest of the proposal, so I'll assume this
> is a
> mistake in wording, not the actual design intent. (Or is it?)
It is a mistake in wording.
> Let's assume the intent was to keep members internally overridable by
> default.
> But then why not go the full way? We can achieve the proposal's goals by
> just tweaking
> existing defaults -- there's no need to introduce complicated new
> overridability levels:
>
> "This proposal does not change the rules of class subclassibility. However,
> it introduces the
> (previously implicit) "open" qualifier to explicitly declare an overridable
> member,
> and changes the rules of default overridability as follows:
> - Members of public classes are now final by default.
> - Members of internal and private classes remain overridable ("open") by
> default."
>
> I prefer plan A, but I'd also be OK with this bare-bones proposal.
>
> I'm strongly opposed to plan B (either as stated in the proposal or what (I
> assume
> above) is the intent behind it.)
Thank you, that's what we're looking for.
> 3. The code examples are needlessly complicated by trying to describe both
> sub-proposals.
> I find this is confusing and it obscures the actual effect of both variants.
I'm sorry, I didn't have much of a choice about this. Perhaps I could have
broken it out
into two completely different code examples.
> The examples are also inconsistent with the text: e.g., AFAICT, property
> `size` below
> is still intended to be overridable inside the module that defines
> `SubclassableParentClass`.
>
> open class SubclassableParentClass {
> // This property is not overridable.
> public var size : Int
> ...
> }
Yes, this should read "not overridable outside of the current module".
> 4. The previous revisions ignored dynamic members, which wasn't great. The
> current
> document acknowleges that "dynamic" members exists, but then *prohibits*
> them from being overridden across a module boundary!
>
> "`open` is not permitted on declarations that are explicitly `final` or
> `dynamic`."
> "A class member that is not `open` cannot be overridden outside of the
> current module."
>
> If this was intentional, I'd love to see the reasoning behind this decision.
> Otherwise the wording should be fixed:
>
> "A class member that is not `open` or `dynamic` cannot be overridden
> outside
> of the current module."
Yes, sorry, this was a drafting error again.
> 5. It seems strangely inconsistent that `open` now implies `public` by
> default,
> but `final` doesn't. The proposal fails to explain this inconsistency.
> Was this point considered and dismissed, or was it not considered at all?
I think the best model of thinking about "open" is that it is an access level
above "public".
The idea of allowing "internal open" came out of discussion but in retrospect
does not
seem to hold its own.
> Changing `final` to imply `public` later would be a source-breaking change.
> Given that SE-0117 is one of the last proposals where maintaining source
> compatibility isn't a major consideration, it's unclear if we'll ever have an
> opportunity to fix such minor inconsistencies.
>
> The same argument also applies to `dynamic`. If it's OK for `open func` to
> imply public visibility, wouldn't it follow from same logic that we should
> treat `final func` and `dynamic func` the same way?
>
>
> 6. The proposal does not suggest an explicit spelling for the new default
> level
> of internal-only overridability for public classes and members.
>
> We can add an optional contextual keyword later, in an additive proposal.
> However, failing to mention this point makes me question if it was
> intentionally
> omitted to prevent bikeshedding, or just forgotten.
>
>
> 7. "`open` is permitted only on class declarations and overridable class
> members
> (i.e. var, func, and subscript)."
>
> Presumably this list is to be taken to include `class func`, but not `static
> func`.
> Or would `class func` remain open by default?
"class" members are still members. The model for "static" on class members has
always
been that they are implicitly final.
> 8. The opening clause in the "open class members" section makes no sense to
> me.
>
> "A class member that overrides an open class member must be explicitly
> declared open
> unless it is explicitly final or it is a member of a final or non-public
> class.
> In any case, it is considered open."
>
> So, AFAICU, when a public member of a public open class overrides a
> superclass member,
> we will have to write either "open override" or "final override". Temporary
> restriction, fine.
> (Shouldn't "dynamic override" be a thing, though?)
>
> But if these are "considered open in any case", why force us to add
> misleading boilerplate?
> What does it mean for a member of a final class to be "considered open"?
Today, you would have to write "public override" on such a member. It is not
added
boilerplate to say that you have to instead write "open override" unless your
intent is to
close off overriding (in which case you have to write "public final override").
> 9. I have trouble interpreting the parenthesized statement in the following
> clause:
>
> "`open` is not permitted on declarations that are explicitly `final` or
> `dynamic`.
> (Note that it's okay if one or both of the modifiers are implicitly
> inferred.)"
>
> Can "dynamic" ever be implicitly inferred?
It's inherited. I can't remember if we require it to be explicit on the
override.
> Can both "dynamic" and "final" ever apply at once?
This is currently forbidden when explicit, but you can declare a dynamic member
of a final class.
> I assume "it's okay" here means `open` is allowed but ignored, in either case.
> Is that right?
Probably the right rule is that open can be subsumed by dynamic, so that you
can override
an open member and make it dynamic. I'm not sure whether you should be able to
say just
"open" on an override of a dynamic member when it clearly remains dynamic.
Inherited open-ness will be ignored on a final class or member.
Honestly, this is why I don't like to include this level of detail in
proposals. People complain if
it's not there, but including it just invites a bunch of complaints about every
detail and distracts
from the important parts of the discussion.
John.
>
>
>
> Ayayay, so much drama!
>
>
>> * 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?
>
> See previous reviews.
>
>> * How much effort did you put into your review? A glance, a quick
>> reading, or an in-depth study?
>
> Probably more than it deserves? ;-)
>
> --
> Károly
> @lorentey
>
>
> _______________________________________________
> swift-evolution mailing list
> [email protected]
> https://lists.swift.org/mailman/listinfo/swift-evolution
_______________________________________________
swift-evolution mailing list
[email protected]
https://lists.swift.org/mailman/listinfo/swift-evolution
