> On Dec 24, 2015, at 10:42 PM, Chris Lattner <[email protected]> wrote: > >> On Dec 23, 2015, at 9:25 AM, Matthew Johnson <[email protected] >> <mailto:[email protected]>> 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:
Thanks for continuing the discussion. I have updated the proposal to reflect the core functionality and moved everything else to the future enhancements section. I think this draft is ready or nearly ready for a PR. Here is a link to the current draft: https://github.com/anandabits/swift-evolution/edit/flexible-memberwise-initialization/proposals/NNNN-flexible-memberwise-initialization.md <https://github.com/anandabits/swift-evolution/edit/flexible-memberwise-initialization/proposals/NNNN-flexible-memberwise-initialization.md> Here is a link to the commit diff: https://github.com/anandabits/swift-evolution/commit/f15360d2e201709640f9137d86a8b705a07b5466?short_path=f5ec377#diff-f5ec377f4782587684c5732547456b70 <https://github.com/anandabits/swift-evolution/commit/f15360d2e201709640f9137d86a8b705a07b5466?short_path=f5ec377#diff-f5ec377f4782587684c5732547456b70> > > "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? This section wasn’t as clearly written as it could be. I revised it in a way that is more clear and in-line with the proposal. > >>> 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. That’s interesting. I have never encountered a language that optimizes away storage for instance variables before so IMO I need to “know too much” about the implementation for it to be reasonable to use an instance member in this way (i.e. without wasting space). > It is also annoying that writing it as a static property would force you to > write something like “X.a" instead of just “a”. I agree with this. I can accept an argument that this provides enough value to avoid changing the language. That said, I do think default values for let properties are higher value. If I had to pick one it would be default values and I would consider it to be worthy of a breaking change in the language. But It would be great if we can find a way to support both. > > >>> 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 don’t love it either but I do think supporting default values for `let` properties is really important. I would like to continue exploring how we can do this. I haven’t come up with anything I like better if we’re not going to change the behavior of initial values. > > 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. Fair enough. I kind of expected you to say this. IMO supporting default values for let properties is super important because it reduces the burden of using immutable members. But I don’t want it to sidetrack the core functionality and will be happy to address it later as an independent addition. I apologize if I’ve pushed on some things more than necessary. The incremental approach makes a lot of sense. I’ll try to be better about adopting it from the beginning in the future. The “future enhancements” section is a good way to present a more complete vision without forcing adoption of it all at once. > >>> 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. If you’re willing, I’d like to continue discussion around this targeting a possible future enhancement. I know it needs to be carefully considered but I do think it is reasonably likely that we can find adequate solutions. I may not have articulated it clearly here, but the derived class does not need to know about its base class properties in general. The basic mechanism I envision is: 1. The call to a super initializer or designated initializer (from a convenience initializer) must be unambiguous on its own (prior to forwarding). 2. When synthesizing memberwise parameters corresponding the base class parameters and forwarding arguments the compiler only needs to look at the base class initialized that was unambiguously determined already. One complication is that the compiler needs to know which parameters to the base class initializer are eligible for forwarding. Originally I was thinking this would be straightforward as it would only happen if the base class initializer was itself a memberwise initializer and the compiler would know which parameters were synthesized for memberwise initialization. I realize now that this isn’t going to be acceptable because it doesn’t retain the “pure sugar” status of memberwise initializers, allowing equivalent manual implementations. This is a very important point I had overlooked. You may be right that thinking in terms of a more general parameter forwarding feature might be a good direction to consider. One interesting twist in this case is that the parameter set published for forwarding by the forwarder would need to be specified by the forwardee and may be a subset of the parameters for both. Another interesting twist in the memberwise initialization case is that I think we would need to apply access control rules to restrict the set of forwarded parameters, but fortunately this would only need to happen when forwarding within a module as public memberwise initializers in other modules would only have synthesized parameters for public properties. I will continue thinking about how this might be solved and also about other cases where such a forwarding feature might be useful. > > > 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. Yep, the same issues apply as with inherited initializers. If we identify a suitable forwarding mechanism I believe it would work equally well for convenience initializers. All of the above aside, if we can’t identify a general solution and were only able to support chained memberwise initializers (derived, convenience, etc) in the same module I think that would be much better than nothing as an eventual final state of memberwise initialization. > > >> 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. Great! I believe I follow and I hope my responses above made sense. >> >> 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 :-) I would like to see a future where we have a more robust alternative for every use case, but until that future arrives I think they are worth supporting if an adequate design can be identified. > >> >>> 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. The final draft of this proposal is focused in that way. At the same time, I hope we can continue to explore whether future enhancements to the feature might allow it to be more powerful. > 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. I agree with this and wouldn’t expect *all* of the superclass properties to be exposed. The access control eligibility rule in the current proposal addresses that as well as the `@nomemberwise` enhancement. I would only expect to see a subset of the superclass properties exposed via memberwise initialization. > >> 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. It feels really good to be putting forward something you’re so excited about! :) > > > 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. I’ll update that section to reflect these comments. One question I have is what the implicit memberwise initializer should do in the case of private members. If we make it follow the rules of this proposal we would break existing structs with private members that are currently receiving the implicit memberwise initializer. I think this would be a good breaking change for both consistency with this proposal and because implicitly exposing private members via the initializer was a questionable choice. A mechanical migration could generate code to add an explicit implementation of the previously implicit initializer that doesn’t qualify under the rules of the new proposal. How do you feel about this? > > Thanks again for pushing this forward, you can also put me down as the review > manager if you’d like. > You’re very welcome. It’s a privilege to be able to contribute ideas, have them taken seriously and hopefully see them lead to progress in the language. I’ve really enjoyed the process and discussions with the core team as well as the broader community. It’s really incredible to see the Swift team embrace the community so openly and so graciously! Merry Christmas! Matthew
_______________________________________________ swift-evolution mailing list [email protected] https://lists.swift.org/mailman/listinfo/swift-evolution
