+1 thanks for your response Chris, it seems to me that it addresses the need this PR is trying to address and it does make it so in a scalable way. I would strongly hope your feedback is added to the proposal and shapes the final solution.
Sent from my iPhone > On 21 Dec 2017, at 07:14, Chris Lattner via swift-evolution > <swift-evolution@swift.org> wrote: > >> On Dec 20, 2017, at 4:19 PM, Ted Kremenek <kreme...@apple.com> wrote: >> >> The review of "SE-0193 - Cross-module inlining and specialization" begins >> now and runs through January 5, 2018. >> >> The proposal is available here: >> >> https://github.com/apple/swift-evolution/blob/master/proposals/0193-cross-module-inlining-and-specialization.md >> When reviewing a proposal, here are some questions to consider: >> >> What is your evaluation of the proposal? >> > I am hugely supportive of the features that these attributes enable, but I > think that the spelling of this is absolutely wrong, and I’m disappointed > that the extensive discussion we’ve had for months about this didn’t make it > into (at least) the alternatives considered section. Here are my concerns: > > Availability Ranges > > Both of these attributes will (perhaps not for Swift 5 given the fact that > these will be new then, but certainly in 5.1 or 6) need to be qualified by > deployment modifiers. We’ll need the ability to specify not just that a > declaration is inlinable or abipublic, but in *which versions* of the binary > package (that they are defined in) have this property. > > For example, while perhaps it will be common for a decl to be “born > inlinable” and just need the form of attribute as specified here, it is just > as clear that this is not the *only* model we need. It is entirely > reasonable (and will be important in the future) to say that something > “became ABI public in iOS14, became abiPublic in iOS 15, and became inlinable > in iOS16”. The use of this will be relatively rare, but it is important for > the model to support this in time. > > Because of this, if we accept the spelling as proposed in this proposal, > these attributes will need to be generalized to have an availability range, > e.g.: > > @abipublic(iOS 15, *) > > The concern is that this proposal opens the door to have a family of > attributes each of which have availability information on them, and this > “family” of attributes will have nothing tying them together into a unified > framework. > > > Pollution of the Attribute Namespace > > Furthermore, these two attributes are the tip of the iceberg, and the core > team has spent a lot of time recently discussing the fact there are > potentially going to be about a dozen attributes similar to these > (fixed_contents, global_var_is_directly_addressible, …) that will only be > required for binary frameworks. It is possible that @inlinable will be > prominent enough to be a global attribute (I personally am not sure if it > will be commonly used or not, it depends a lot on how widely used binary > frameworks are). That said, it is clear @abiPublic will not be commonly > used, and many attributes that follow these will be even more obscure. > > This is bad for three reasons: > > 1) we’re polluting the general attribute namespace with obscure things. > Pollution of the attribute namespace may have a marginal impact today, but > will start to matter if/when we ever get user defined attributes. > > 2) The other reason is that this provides no general framework to tie > together these things that affect binary frameworks into a unified framework. > > > 3) Furthermore, I don’t like attributes being a dumping ground for weird > performance hacks required by binary frameworks. It is a practical necessity > that we support these because they are extremely important for narrow cases, > but we don’t need to put them into a syntactically prominent spot in the > grammar. > > The name “ABI” > > A minor point, but the specific name “abiPublic” is not great in my opinion, > because “ABI” is a term of art for compiler hackers. Most users have no idea > what ABI means, and those who think they do often don’t. Very few people > really understand what “stable ABI” means for example. > > It would be better to go with something like “apiPublic” or “symbolPublic” or > “linkableButNotAccessible” or something else long. This will not be commonly > used in user code, so being long and descriptive is a good thing. > > > Counterproposal: > > There is a simple way to address the two concerns above: we already have a > framework for handling API evolution with binary frameworks, the @available > attribute. We can spell these “attributes” as: > > @available(inlinable) // this symbol has been inlinable since it was > introduced > > which generalizes properly when we add version ranges: > > @available(iOS 14, *) // this was introduced in iOS 14 > @available(linkerSymbol: iOS 15, *) // this decl’s symbol became > “abiPublic" in iOS 15 > @available(inlinable: iOS 16, *) // this decl became inlinable in iOS > 16 > public func foo() {… } > > and allows us to bury weird hacks like “abiPublic” and the other even more > obscure things that are coming outside of the global attribute namespace: > > @available(global_var_is_directly_accessible: iOS 15, *) > public var myDispatchOnceToken : ... > > > Given this unified framework for handling ABI evolution, we can then > separately discuss which ones of these proposals are common and important > enough to sugar into a top level attribute. For example, given the general > model for inlinable above, we could then (possibly as a later proposal) > introduce: > > @inlinable // this symbol has been inlinable since it was introduced > public func foo() > > as sugar for: > > @available(inlinable) > public func foo() > > … which means that the sugar forms can be separately debated, and that the > sugar forms don’t have to permit the full complexity of the general case (the > availability list). It still isn’t clear to me whether @inlinable meets the > bar to be a global attribute, I can see both sides of that argument, and it > seems valuable to be able to separate the engineering work to introduce the > feature from the bikeshed discussion about whether it should be sugared or > not. > > > In short, respectfully request that you at least add this approach to the > "alternatives considered” section. I also suggest you strongly consider > pursuing this direction. It solves the same problem as your proposal but: > > - scales better as we add more “attributes" in the future - which will be of > increasingly narrow applicability. > - provides a unifying model for all of the binary framework hints > - puts all the availability markup into the feature we already have for this. > - provides a better naming framework for things like abiPublic, because you > can say "@available(linkerSymbol)” to say that this is making the linker > symbol available from the binary framework. > > -Chris > > > > > > > >> >> 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? >> >> Thanks, >> Ted Kremenek >> Review Manager >> >> _______________________________________________ >> swift-evolution-announce mailing list >> swift-evolution-annou...@swift.org >> https://lists.swift.org/mailman/listinfo/swift-evolution-announce > > _______________________________________________ > 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
