+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 

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

Reply via email to