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 the review manager. When emailing the review manager directly, please keep the proposal link at the top of the message.
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
As someone that has been maintaining and quite decent Objective-C codebase while migrating it to Swift this is a very welcome addition. A little bit late for me but I wished I had this a few years back. Huge win.
ObjC forward decls is the biggest pain point I have experienced in migrating code bases, and while this won't magically solve everything it is a giant step forward. It is a shame it adds another place the REPL diverges from "normal compiled well ahead of time" Swift, but that is a small price to pay for the improvement.
Does this require a Swift language change? Or does Swift already let you assign instances of unavailable types, as long as you son’s try to create them yourself?
Can’t I pass an NSProxy instance to Swift?
In GameController.framework we use a fair number of non-NSObject-conforming protocols. I’m personally OK with having to add NSObject conformance to these protocols, but the importer should expect to resolve forward declarations to non-NSObject-conforming protocols. It should probably continue to emit the unavailable shadow protocol declaration.
Perhaps, but correct me if I'm wrong in understanding that NSProxy instances will implement the methods provided by NSObject anyways. I will run a test but I think so long as instances respond to messages that NSObject guarantees, it should be fine. What would be problematic is if there is a legitimate use case of importing a type that does not respond to messages that NSObject does.
The trouble is we have no way of knowing by inspecting a forward declared protocol what protocols it conforms to. We either overly optimistically assume all protocols conform to NSObject, or conservatively make no such assumption. If we get the assumption incorrect, the Swift type checker would allow you to call methods that the Objective-C implementation can't handle, resulting in runtime message receipt failure. It is a trade off either way, but I would be interested in what the community would prefer.
The flag is always disabled in the REPL, as allowing it currently leads to confusing behavior. In the REPL, the environment in terms of whether a complete definition of some type Foo is available can change during execution.
I’m going to repeat my caution from the pitch thread here because I think it’s important: the “REPL” includes the LLDB expression evaluator. A feature is being proposed to make it possible to write code in Swift source files that you will not be able to write in LLDB, even when stopped in a function that uses the feature.
Below the philosophical issue, there are a number of details to work out even in the proposed solution. What happens when you try to print a local variable of this type using the expression evaluator? What happens when LLDB tries to load the SIL for an inlinable Swift-defined method that manipulates one of these types, if the type itself cannot be loaded?
That said, it would be great if this feature could come to the REPL eventually, but the REPL should not stand in the way of progress in the compiled experience.
This is something the core team should rule on, but I personally think the debugging experience should stand in the way of improving the compiler experience. It is bad enough that features in the compiler have (and always have, in this project’s history) outpaced debugger support, but to introduce a feature with no plan for debugger support goes too far, in my opinion. (I admit I’m at fault for some of the past situations in which the compiler outran the debugger, but there was always an idea of how the debugger would eventually implement the feature.)
I agree that there’s room for (significant) improvement around forward-declared ObjC types, and the proposal author has put a lot of thought into how to carefully introduce them without disrupting the rest of the language. But I cannot endorse this as a solution; it is, no pun intended, incomplete.
I think it would make more sense to have the ClangImporter pull the types in as unavailable typealiases of Any. Parametric usages in functions would still appear as Forward!, and parametric usages in types as T<Forward>. I believe this would hew closer to the id-as-Any model.
// SomeClangModule
@available(*, unavailable, message: "This Objective-C class has only been forward declared; import its owning module to use it")
public typealias ForwardClass = Any
@available(*, unavailable, message: "This Objective-C protocol has only been forward declared; import its owning module to use it")
public typealias ForwardProtocol = Any
public class Bar {
@available(*, unavailable, message: "This method references an Objective-C class 'ForwardClass' that has only been forward declared; import its owning module to use it.")
public func foo(one: ForwardClass!, many: [ForwardClass]) {}
}
@available(*, unavailable, message: "This extension references an Objective-C protocol 'ForwardProtocol' that has only been forward declared; import its owning module to use it.")
extension Bar: ForwardProtocol { }
Overall, I appreciate the effort to try to improve the diagnostics for end users in such a common case. The Clang Importer already has a history of trying to treat incomplete types with some care (cf OpaquePointer for existing forward declarations) and this seems like a nice extension. That said, I think the proposal as it stands could use a little more discussion. I'm particularly nervous about the idea that import order can affect the API surface presented to and exported by a module or file. This will have impacts on the incremental build that should be explored.
Fair enough, I can appreciate the concern for the debugging experience, and I too defer to the core team on that ruling.
As I mentioned, we can enable this in the LLDB expression evaluator but as it stands will have to live with a certain amount of incompatibility between the incomplete and complete notions of the type. Given the unavailability of the synthesized types, the problematic scenarios are in my eyes, quite rare, but existent nonetheless.
In general, I am less familiar with the ASTContext LLDB operates on and how it invokes the ClangImporter, and so would appreciate any suggestion from those more familiar.
Could you elaborate a little on why you believe that to be a better choice? It seems to me you would be losing a considerable amount of type safety. That said, I could see the use of Any reducing some of the issues with LLDB conflicts.
Why do the users of the incomplete type need to be marked unavailable? The purpose of this proposal is to make them available from Swift, despite depending on an incomplete type.
For one the API surface exported by a module or file should not be influenced by this change, since synthesized types are unavailable and thus should not escape file scope. Furthermore import order also shouldn't have an effect (REPL / LLDB aside). Please let me know if I've misunderstood something.
Not completely important for the points you are making, and I might be wrong, but the code
extension Bar: ForwardProtocol { }
should not be possible with a forward declared protocol. If I remember correctly, the Obj-C compiler will need a protocol conformance to be done against a fully declared protocol.
This already happens with the current importer: there are cases in which importing a module that provides a fully declared type, and later importing a module with forward declarations of the same type resolves the type and import functions and methods that use the type, while the opposite import order might not make the same methods visible to Swift. One of the ideas behind this proposal is trying to give the user visibility into cases like the later.
Declaring an interface is different (in ObjC) from declaring an implementation, because the latter has to generate code. Is that a warning or an error in Clang?
This only occurs in REPL contexts, and only if you try to use the declarations between module imports (otherwise they are processed together before any APIs get imported)…but it is a good example of a feature that is best-effort in REPLs even though the compiler can do better.
Yes, it has to generate metadata and code for the reference to the protocol metadata for P. The forward declaration provides enough information to do just that. I suppose that also defeats my proposal for Any unless we do some heroics to emit metadata for any forward-declared referenced protocol descriptors.
I think I almost never use the REPL (besides from time to time in LLDB), so my experiences are mainly using the compiler, and I can say that I have seen cases where import Complete; import Forwarded; imports all pieces of Forwarded including usages of forwarded types, while import Forwarded; import Complete; does not. I have never been able to figure out exactly what makes the Complete, Forwarded order works sometimes, and not others (I am pretty sure I have never seen Forwarded, Complete order work). I know it does not make sense, but it is like the environment that the modules get imported into is influencing what the module is seeing (which should not be how modules work, but it is how I have seen them work).
+1, this is a very much needed change for quality of life in mixed code based and build times.
I’m also not sure if Objc types visible to Swift must inherit from NSObject, but if not then we shouldn’t subclass placeholders from NSObject (or conform to NSObjectProtocol) which will allow for unsafe calls to their methods from the Swift module. I can’t think of the benefit of calling NSObject methods on a placeholder type anyway instead of importing the full module — this feature is generally going to be used for passing through types.
I don’t think we need to block such a huge positive change like this over the REPL, where there is already precedent for incompatibilities (and can be addressed in a follow up).
Classes and protocols forward declarations are not the only forward declarations which need more love from Swift. For example, you can have named enum and named struct forward declarations, like these:
Will this proposal ensure that both swift compiler and debugger are correctly interpreting these forward declarations, too?
If not, is it worth to tackle the problem partially and not holistically? What is the externalised effect onto the later evolution that would attempt to tackle the problem holistically?
I don’t think we need to block such a huge positive change like this over the REPL, where there is already precedent for incompatibilities (and can be addressed in a follow up).
I would argue that the whole ClangImporter is a deferred follow-up (example), which never actually achieved a feature parity with the compiler. It more often fails than succeeds to import mixed modules that were successfully compiled. Adding more to the already existing debt and separation of the language support does not seem like a proper way of addressing the problem.
It does not, but while I haven't tried, I imagine they could be implemented in a similar manner.
Aside from the overhead of another round of evolution, I'm not sure there is a notable cost. Personally I would rather keep the scope small and prove a solution is viable for the first approach rather than trying to tackle them all at once. The remaining forward declarations seem like just another case of the same problem, I don't see why we would have issues tackling them in a follow up pass.