> On Dec 23, 2015, at 9:25 AM, Matthew Johnson <matt...@anandabits.com> wrote:
>> 
>> Hi Matthew,
>> 
>> I continue to really like the approach and direction.  Here’s an attempt to 
>> respond to both of your responses, I hope this comes across somewhat 
>> coherent:
> 
> Excellent, thanks!  I have completed a third draft of the proposal.  It may 
> (probably does) still require further refinement but I believe it is another 
> solid step forward.
> 
> Here’s the complete draft: 
> https://github.com/anandabits/swift-evolution/blob/flexible-memberwise-initialization/proposals/NNNN-flexible-memberwise-initialization.md
>  
> <https://github.com/anandabits/swift-evolution/blob/flexible-memberwise-initialization/proposals/NNNN-flexible-memberwise-initialization.md>
> 
> Here’s the commit diff in case that is more helpful: 
> https://github.com/anandabits/swift-evolution/commit/8287b67569038000aa4231cc7725e5fbeb7fe8ce?short_path=f5ec377#diff-f5ec377f4782587684c5732547456b70
>  
> <https://github.com/anandabits/swift-evolution/commit/8287b67569038000aa4231cc7725e5fbeb7fe8ce?short_path=f5ec377#diff-f5ec377f4782587684c5732547456b70>
> 
> Discussion on a couple of topics continues inline below as well.

Great, comments below:

"memberwise initializer for us in some” -> typo “use”
"elimintate the existing “ -> typo “eliminate"

"Sometimes properties that should be immutable are made mutable” -> should be 
updated to reflect the new approach?

>> There are two major problems:
>> 
>> Problem 1: supporting this would *prevent* us from allowing memberwise 
>> initializers to be public.  A really important feature of the memberwise 
>> design is that it is “just sugar” and that people can write the initializer 
>> out long hand if needed (e.g. if they want to add or reorder members without 
>> breaking API).  With your proposed design, I could write:
>> 
>> public class X {
>>   let a = 42
>>   public memberwise init(...) {}
>> }
>> 
>> and use it with: X(a: 17).  However, there is no way in swift to write that 
>> initializer out longhand.  This is a critical problem to me.
> 
> Maybe it wasn’t clear, but I was suggesting that this particular rule be 
> changed for all initializers.  That would have made it possible to write the 
> initializer out longhand.  But I have pulled that part of the proposal so it 
> doesn’t matter now. 

Sounds good thanks.  Changing the initialization semantics for lets in general 
is something far more nuanced and should be discussed separately.  The design 
we have now is carefully considered, and has already been refined from what 
shipped in Swift 1.0.

>> Problem 2: This can cause very surprising performance issues, because it 
>> forces the let property to be stored.  With the previous example, it is a 
>> goal for us to be able to compile:
>> 
>> public class X {
>>   let a = 42
>> }
>> 
>> into the equivalent of:
>> 
>> public class X {
>>   var a : Int { return 42 }
>> }
>> 
>> because people like to use local lets as manifest constants (avoiding “magic 
>> numbers”).  With your proposal, we’d lose this capability, and we’d have to 
>> store them any time there is a memberwise initializer.
> 
> I would have never considered writing a type with an instance member with 
> constant value that cannot be assigned a different value in the initializer 
> as I would have considered it wasted space and possibly worthy of a compiler 
> error.  I would have written:
> 
> public class X {
>   static let a = 42
> }
> 
> Clearly I was underestimating the optimizer!  I can see value in the ability 
> to refer to the member without a prefix.  Given the (guaranteed?) 
> optimization it definitely makes sense.  

I understand, and I would have written the same.  However, it has been pointed 
out to me numerous times that I only know to write that because I “know too 
much” about the implementation.  It is also annoying that writing it as a 
static property would force you to write something like “X.a" instead of just 
“a".


>> Neither of these problems apply to vars, which is why I think we can support 
>> vars in this model, but not lets.
> 
> As you said you agree with the desire to support this behavior, maybe you 
> would be comfortable with a different approach.  Rather than using the 
> initial value we could use an attribute:
> 
> public class X {
>   @default(42) let a
> }
> 
> This would allow us to still support the desired memberwise initialization 
> behavior without conflicting with the current “initial value” behavior.  I’m 
> have updated the proposal to specify the behavior this way.  If you don’t 
> like it let’s continue the discussion.
> 
> It could be called @default, @memberdefault, @memberwisedefault, etc.  The 
> name doesn’t really matter to me.

I’m not inclined to like an approach that requires magic attributes to be 
sprinkled around the code.  The more attributes and annotations you have to add 
to your properties, the less the syntactic sugar of memberwise initializers are 
paying for themselves.

I see that your new version introduces the @default attribute.  I’d strongly 
suggest the same advice as before (I know I must sound monotonous :-) - please 
start your proposal minimal and straight-forward, by moving @default to the 
“possible future extensions” section.  We can always add it later to build out 
the model if there is a strong need, but I’d rather see us roll out the next 
incremental step and learn from it before we do.  That allows us to carefully 
consider how much each piece is adding and costing as independent entities.

>> I understand, but it is a pure extension to the basic proposal.  The 
>> proposal is complex enough as it is, so inessential parts should be split 
>> out for discussion and implementation.  I’m not saying that we shouldn’t do 
>> @nomemberwise in (e.g.) the swift 3 timeframe, I’m saying that it should be 
>> a separate discussion informed by the design and implementation process of 
>> the base proposal.
> 
> That makes sense.  I have updated the proposal to reflect this.  Incremental 
> change, right? :)

Incremental is good!

> 
> I absolutely agree that the compiler should not be in the business of making 
> a guess about what superclass initializer needs to be called!  

Pwew, great!

> The example was intended to include a call to provide all of the arguments 
> necessary to disambiguate prior to memberwise argument forwarding.  In this 
> case of the present example none are necessary.  I am including the corrected 
> example here and have also updated the proposal.  It should have read like 
> this:
> 
> class Base {
>     let baseProperty: String
> 
>     // user declares:
>     memberwise init(...) {}
>     // compiler synthesizes:
>     init(baseProperty: String) {
>         self.baseProperty = baseProperty
>     }
> }
Looks good so far.

> 
> class Derived: Base {
>     let derivedProperty: Int
> 
>     // user declares:
>     memberwise init(...) { super.init(...) }
>     // compiler synthesizes (adding forwarded memberwise parameters):
>     init(baseProperty: String, derivedProperty: Int) {
>         self.derivedProperry = derivedProperty
>         super.init(baseProperty: baseProperty)
>     }
> }
This I still have a concern with, in two ways:

1) This still has tight coupling between the base and derived class.  
Properties in a based class are not knowable by a derived class in general 
(e.g. across module boundaries) and this directly runs afoul of our resilience 
plans.  Specifically, across module boundaries, properties can change from 
stored to computed (or back) without breaking clients.

2) You’re introducing another unnecessary feature "super.init(…)” which will 
have to be independently justified. 

I consider #1 a showstopper but, in any case, this is an orthogonal add-on to 
your base proposal, which can be considered independently as the base proposal 
rolls out.  I’d strongly suggest dropping it from the first revision of the 
proposal.  It can be added at any time if there is a compelling need.  If/when 
we consider this, I’d suggest thinking about it in terms of a more general 
parameter forwarding feature, instead of something tied to memberwise 
initializers.


A related concern is the part of your proposal that supports memberwise 
initializers for convenience initializers.  I think this should be dropped for 
a number of reasons:

1) Again, it is a major problem for resilience, because convenience 
initializers (unlike designated ones) can be added to a type in an extension.  
That extension can be defined in another module, and it isn’t knowable it that 
module what stored properties a type has.

2) Convenience initializers, by intent if not "by definition”, should not 
publish every little detail of a type.  They are intended to be used as an 
additional non-canonical way to initialize a type.  While I’m sure there are 
some exceptions, the ability to have a memberwise initializer doesn’t seem very 
core to them.


> I understand there also potentially resilience concerns around supporting 
> inheritance.  If you’re willing to dive into specifics I may (or may not) be 
> able to find solutions.  If I am not able to identify solutions, at least I 
> have tried and understand the specific issues involved.  :)

I’m definitely willing to explain, please let me know if the above makes sense 
or not.
> 
> I personally don’t like inheritance very much and consider it a tool of last 
> resort, but I do think it is best for a feature like flexible memberwise 
> initialization in a language that offers inheritance should support it if at 
> all possible.

Ok, if you don’t find derived classes to be super important, then it probably 
isn’t worth hyper-optimizing :-)

> 
>> Instead, the user should have to write:
>> 
>> memberwise init(baseProperty : Int, ...) {
>>   super.init(baseProperty: baseProperty)
>> }
>> 
>> This doesn’t reduce the utility of this feature, and it preserves 
>> separability of class from superclass.
> 
> 
> Ideally we could avoid writing this manually.  We still have an M x N problem 
> (M inherited properties, N subclass initializers).  
> 
> Writing that manually would’t reduce the utility of the feature in cases 
> where it is applicable, but it does severely limit its applicability to 
> inheritance hierarchies.

I don’t agree.  The ability to manually add parameters to the memberwise 
initializer, along with the extant requirement to explicitly call super.init 
seems to directly solve this.  This feature is about automating initialization 
of a single classes properties independent of the superclass.  Also, it is very 
uncommon for a class to expose an initializer for *all* of its properties if it 
is intended to be subclassed anyway.

> You’re welcome!  I know this proposal is “just” syntactic sugar, but I 
> believe it will have an impact on initialization designs in practice.

Yes, I agree, I’m very excited about this for two reasons: 1) I think it is 
will be highly impactful for all kinds of code written in swift, and 2) it 
cleans up a half-baked area of the language by replacing it with some thing 
much better thought out.


Other comments:

In "Impact on existing code”, given your proposal, I think that we should keep 
the implicit memberwise initializer on classes, start generating it for root 
classes, and generate it for derived classes whose parent has a DI with no 
arguments (e.g. subclasses of NSObject).  We should keep the current behavior 
where it is generated with internal behavior, and it is surpressed if *any* 
initializers are defined inside of the type.

Thanks again for pushing this forward, you can also put me down as the review 
manager if you’d like.

-Chris

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

Reply via email to