Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager by DM. When contacting the review manager directly, please put "SE-0496" in the subject line.
What goes into a review?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift. When writing your review, here are some questions you might want to answer in your review:
What is your evaluation of the proposal?
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?
More information about the Swift evolution process is available at:
The proposal mentions @_alwaysEmitIntoClient, but only in passing in one of the code examples when discussing interactions with @inlinable. Even though @_alwaysEmitIntoClient is an underscore’d attribute, can the proposal add more clarity about how we should expect this new attribute to interact with it?
Edit: Never mind, I see the other pitch threads related to this and @_alwaysEmitIntoClient. So instead, I would just ask if we can clarify or remove the existing reference to @_alwaysEmitIntoClient in this current proposal. Thanks!
Broadly this feels reasonable to me, but the proposal neglects to say what is going to happen to the existing double-underscore spelling. It isn’t stable, but it is useful to know whether the intention is that it will be immediately removed, or not.
__always has different semantics than the proposed always (in particular, __always does not apply in Debug) so it cannot simply be removed in favor of the new annotation.
Are there any plans to make possible using of @inline(always) at call site for usableFromInline declarations. Currently we have 2 options:
mark something as inlineable and compiler automatically chooses will the function be inlined or not
@inline(always) – will force to inline everywhere (except cases described in proposal)
Bat there is no option to have inlineable function, which can be fine in most cases with default compiler inlining decisions, but needed to always be inlined in concrete place.
Call-site inline control is definitely something that a number of people (including myself) are interested in, but would be a separate proposal.
Arnold's thread "Optimization Controls and Optimization Hints" (Optimization Controls and Optimization Hints) has a high-level overview of various ideas in this area and would be a good place to continue that discussion.
There is a tangentially related point—and sorry if this is in the final proposal and I’ve missed it—that it would make sense that calling @inline(always) functions of any visibility should be permitted from inlinable function bodies. That is, even private inline-always functions should be “usable from inline” (albeit in a totally different sense wrt how it’s usable).
The proposal states that public/open/package @inline(always) imply @inlinable semantics but not for internal and lower access levels. That is intentional.
If we imply _alwaysEmitIntoClient semantics on internal functions it would entail that any symbols they reference would have to be usable from inline. That would limit the applicability. (Edit:) It would limit the applicability on internal functions that are not otherwise used from @inlinable contexts.
The end result does seem unduly restrictive either way, though. Consider the following example:
@inlinable
public func f() {
g()
// error: global function 'g()' is private and cannot be referenced from an '@inlinable' function
}
private func g() {
print("Hello, world!")
}
I think it would be reasonable for a user who has internalized (ha) that the semantics of @inline(always) mean direct calls are guaranteed to be inlined, to expect that they should be permitted to call g() if it is marked @inline(always). After all, if I can "manually" inline the body into f(), and there is an attribute that purportedly directs the compiler to do the same thing, it ought to work exactly as advertised.
Your explanation makes sense as to why the restriction, but I'm wondering if this is a hint that something about this design isn't composing as well as it could be with the language as it is.
This is the consequence of the application of existing rules for library evolution: you can't reference private/internal(-and-not-usable-from-inline) declarations from "inlinable" contexts.
In my opinion: It would be very confusing to make an exception for function bodies that don't reference anything that needs to be made usable from inline.
The point I am trying to make is that we the following does not work today as you mentioned:
@inlinable
public func f() {
g()
// error: global function 'g()' is private and cannot be referenced from an '@inlinable' function
}
private func g() {
print("Hello, world!")
}
Adding the optimization control to inline should not change that:
@inlinable
public func f() {
g() // now works?
}
@inline(always)
private func g() {
print("Hello, world!")
}
If we remove the control it stops working again. That would be more confusing to me.
Yes—aware and agree—but in the scenario above, the behavior that falls out of composing these rules is incongruous with its stated semantics. I regard this as a hint that somewhere among the existing rules and/or proposed rules there's a choice or two that could or even should be revisited to make it behave differently. Maybe there isn't an easy way to square the circle, but first I'd like to see if we can agree that there is a square and a circle.
Yeah, I'd disagree with that: indeed, that's the premise of my feedback.
From the surface semantics we're exposing to Swift users, it makes a lot of sense that we can't call private g() from inside inlinable f(), since f() might have to call g() from across a module boundary. If the semantics and indeed purpose of @inline(always) are to guarantee that a direct call will be inlined (or to get an error), then it is reasonable for a user to wonder: how could adding that attribute not be one of the recommended ways of resolving the existing error? After all, aren't we telling users that it precisely will ensure that there won't be a call across a module boundary?
While I think that the thing you’re poking at is a valid to direction to explore, I also think it’s likely to be a can of worms and is completely separable from this proposal. If you make the example more complex I think it becomes clear that it is not easy to define how the type checker can allow what you are proposing in the general case without more explicit annotations:
In this more complicated example, g()now returns a type from the privately imported module Foo. That import is mean to be hidden from module clients, so the semantics you’re proposing cannot work in this example, at least in the case of library evolution being enabled since the private import will not be exposed in the module interface.
In order to achieve this, there needs to be a way for the compiler to understand that there is a closure of declarations that have a non-public access level but are actually exposed across the module boundary for the purposes of exporting implementations to clients. That attribute already exists: it’s @usableFromInline. It is not compatible with private declarations today, but it is compatible with internal declarations and that’s what I would expect a developer to use instead in this circumstance.
I think it would be reasonable to expand @usableFromInline so that it also supports private. I don’t think there’s anything about this proposal that necessitates solving that problem simultaneously, though; I’d rather see that proposed separately.
Yes, given Arnold’s excellent points, I do suspect that we will need other annotations to make it work—for the moment I’d want us to be sufficiently comfortable that it is completely separable and that the current feature design doesn’t make choices that unwittingly foreclose that future work.
There are a number of complexities here to the extent I’m probably going to be stumped for a while.
That said, does this example reveal some other restrictions necessary for the present feature? Suppose in the same file as this private import, I have an @inline(always) internal function that invokes your g(). Is that allowed under the current proposed design? If so, doesn’t that also leak the import outside the intended scope?
The ability to export the function body to the importing module: facilitated by the attributes @_alwaysEmitIntoClient and @inlinable
The optimization of expanding a function body into a call site: faciliated by the optimization control @inline(always)
If there is an @inlinable function that references symbol g, this symbol g either needs to have an externally available ABI entry point (i.e a high enough access level, or be annotated with @usableFromInline) or needs to be exporting its function body (@inlinable/@_alwaysEmitIntoClient).
If the user wants to make the example below work, the user needs to choose one of the two: @usableFromInline or @_alwaysEmitIntoClient.
The choice to further cause the optimization of expanding the function body is an orthogonal choice that is not needed to solve the problem. Rather, it is an optimization on top of it. If the user desires for the declaration to be inlined, they first need to mark it @_alwaysEmitIntoClient.
The answer to the question is: no the purpose of the attribute is not to solve the problem but to force an optimization and sometimes further prerequisites need to be met and we will emit an error if they are not.
If the internal/private function references internal symbols than existing library evolution rules apply to deal with that, e.g making internal types @usableFromInline.
@inlinable
public func f() {
g()
}
@usableFromInline
struct InternalType {}
@_alwayEmitIntoClient
internal g() {
var x = InternalType()
print(x)
}
Can't we tie the two concepts _alwaysEmitIntoClient/inline(always together?
The constraint that g() needs to be @_alwaysEmitIntoClient such that we can facilitate the @inline(always) annotation is imposed by the module local @inlinable annotation. We can reliably detect this an alert the user with an error. This is true because @internal/private declarations can only be referenced by source code that is local to the module.
If we imply _alwaysEmitIntoClient from @inline(always) for internal/private declarations then we either force the user to mark reachable types from those declarations as externally available (e.g @usableFromInline) or preclude the usage of the attribute.
This is undesirable in my opinion as not all internal declarations are referenced from "@inlinable" code.
public func nonInlinable() {
g()
}
@inline(always) // implies _alwaysEmitIntoClient
internal func g() {
}
// now needs to be @usableFromInline
struct LocalType {}
The proposal specifies that @inlinable is only inferred for public/package/open access levels and not for access levels below that (here and here)
If the user decides to add inline-able semantics to f() that is covered under existing library evolution rules. In the example below, they are not allowed to.
A thought popped into my head today on the way to work:
To my mind, the major advance here is that (1) and (2) are now diagnosed. I think (3) represents the right behavior for such a feature. That (4) doesn't diagnose an error is quite sensible, but I wonder if any consideration was given to adopting an approach here that parallels one that we adopted for unsafe?
That is, consider assignment to a first-class function value:
@inline(always)
func callee() { }
func useFunctionValue() {
let f = callee // function value use, may be inlined but not diagnosed if not
}
Perhaps we could require users to write something like let f = @outlinable callee to acknowledge that this usage may be inlined but may not be. At first blush it seems that a behavior like this would advance the overall thrust of the proposal.