Pitch: Improved ClangImporter Diagnostics

Evolution Pitches
,
1

Hi everyone,

We would like to propose improvements to the ClangImporter to provide more detailed diagnostics when an entity cannot be imported from C or Objective-C. As it stands, when the ClangImporter fails to import an entity (function, type, macro, etc.) either entirely or partially, failure is largely silent. The current Swift compilation errors present largely as if the entity was never defined at all.

We are proposing adding two different kinds of signals to provide more context to the users:

  • Warnings for the different constructs from headers that fail to import.
  • Errors for when those constructs are used in Swift code by generating stubs marked unavailable in place of the not imported entities.

For the time being, we propose to hide this functionality behind a new opt-in flag.

Proof of Concept

Here GitHub - NuriAmari/swift at diagnostics-pitch we have prepared a small proof of concept that centers around a couple of builtin C types with no Swift representation. As it stands, if such a type is used in a C function signature, the function will not be imported by the ClangImporter. Similarly, if an unsupported type is used in a field declaration, the ClangImporter will import the struct, but it will appear as if the field was never declared.

Demonstration

Below we present a minimal example demonstrating the improvements provided by the proof of concept and the kinds of improvements we would like to continue making. Here are the files involved, all in the same directory.

module.map

module complex {
    header "complex.h"
    export *
}

complex.h

struct MyStruct {
float _Complex x;
int y;
};
double _Complex myFunc();

demo.swift

import complex
let _ = myFunc()

demo2.swift

import complex
var s : MyStruct
s.x = 5.0

For the first demonstration, we show the current behaviour before enabling the new diagnostics:

swift-frontend -typecheck -I . demo.swift
demo.swift:3:9: error: cannot find 'myFunc' in scope
let _ = myFunc();
^~~~~~

As mentioned, the function simply can’t be accessed, though the user hasn’t really made any mistakes. After diagnostics are enabled, we get more helpful information:

swift-frontend -typecheck -enable-experimental-clang-importer-diagnostics -I . demo.swift
/Users/nuriamari/git/swift-project/demo/complex.h:6:1: warning: function 'myFunc' not imported: type 'Complex' is unavailable in Swift
double _Complex myFunc();
^
demo.swift:3:9: error: 'myFunc()' is unavailable: type 'Complex' is unavailable in Swift
let _ = myFunc();
^~~~~~
complex.myFunc:2:13: note: 'myFunc()' has been explicitly marked unavailable here
public func myFunc() -> Any
^

Though not shown in this example, the proof of concept also handles invalid parameter types, not just return types.

We provide similar improvements for struct fields:

swift-frontend -typecheck -I . demo2.swift
demo2.swift:4:3: error: value of type 'MyStruct' has no member 'x'
s.x = 5.0
~ ^

Again, the user is left wondering what happened to the x field. After enabling diagnostics, the situation is a little more clear:

swift-frontend -typecheck -enable-experimental-clang-importer-diagnostics -I . demo2.swift
/Users/nuriamari/git/swift-project/demo/complex.h:2:20: warning: field 'x' not imported: type 'Complex' is unavailable in Swift
float _Complex x;
^
demo2.swift:4:3: error: 'x' is unavailable: 'Complex' is unavailable in Swift
s.x = 5.0
^
complex.MyStruct:3:16: note: 'x' has been explicitly marked unavailable here
public var x: Any
^

Admittedly, these examples are quite niche, but they serve to communicate the type of improvements we wish to make.

General Design

The proof of concept above contains two main flows that we propose be repeated where needed to add support for more diagnostics. A pull request has been opened here to highlight the changes: Clang Importer Diagnostics Pitch by NuriAmari · Pull Request #39655 · apple/swift · GitHub.

Diagnostic Reporting

The first flow is responsible for recording diagnostics and surfacing them to the correct code locations. Many methods within the ClangImporter are used to transform a particular Clang AST node and return a pointer to the corresponding Swift AST Node. In the case of import failure, nullptr is returned, leaving little room for communicating diagnostics. In the proof of concept, https://github.com/NuriAmari/swift/blob/diagnostics-pitch/lib/ClangImporter/ImporterImpl.h#L322 we have created a new wrapper structure to include both the Swift AST Node and any diagnostics that may have been produced. We have applied this new return type in a few locations, but would like feedback on the approach as a whole.

We have observed that other additional return values are communicated using by-ref parameters, for example when importing a method the async convention or the error type are returned this way. This option was briefly explored, but found it to be less appealing.

Stub Generation

To construct stubs for unimported components, we have tried as much as possible to reuse the existing AST generation code. As shown in the proof of concept swift/ImportType.cpp at diagnostics-pitch · NuriAmari/swift · GitHub when a small portion of the AST fails to import, we attempt to replace it with a version using Any as early as possible. For example, if a function fails to import due to an unsupported return type, we replace the return type with Any and allow the function generation to proceed as normal otherwise. Before the enclosing AST node is finalized, we check to see if diagnostics were reported, even for a “successful” import, and mark the node unavailable accordingly. We recognize some care must be taken in ensuring that such a replacement does not result in an erroneously imported unsupported entity, but we think reusing the existing code is worth this risk.

Diagnostics + Repeating Name Lookups

While attempting to understand the ClangImporter and how diagnostics are reported, we have come to realize that declaration lookups are often performed repeatedly in a series of increasingly broad searches: swift/PreCheckExpr.cpp at main · apple/swift · GitHub . The extra calls into the ClangImporter result in diagnostics (both new and existing) to be reported either repeatedly or at inappropriate times, particularly for the broader searches (ie. typo search). Thus far, we’ve been able to avoid additional lookups by providing a stub on the first search, but in our investigation of C macros, this hasn’t worked as well. If anyone has experience with this issue, or any suggestions as to a fix, we would appreciate any input.

Feedback / Future Direction

We would appreciate any feedback from those more familiar with the ClangImporter on our general approach. In particular, input from @zoecarver, @egor.zhdan (active in the Clang Importer code), @jrose (listed as the owner of the Clang Importer), and @michael_ilseman, @Douglas_Gregor (worked in the diagnostic code of the Clang Importer) would be wonderful. Also, feel free to bring anyone you think might be interested into the discussion. If the patterns presented in the proof of concept are reasonable, we plan to expand to other hopefully more impactful unimported C/Objective-C constructs. In the immediate future, we plan to tackle the following C constructs:

  • Macros. Only mostly trivial macros are supported currently, which sometimes creates problems importing some headers that rely on macro expansion to work.
  • Forward declared classes. Clang Importer supports some cases of forward declared classes, but when the forward declared class is not defined in the same module, functionality that uses it is not imported. Even importing both modules from Swift will not solve the problem.

That being said, we are actively seeking out more silently unavailable constructs that could use diagnostics. If anyone is aware of more examples, we would greatly appreciate any input and think compilation of such a list would be beneficial, even if we are not able to address every issue.

In some cases, it is complicated to distinguish between what is a bug in the Clang Importer and what is a limitation of the translation mechanisms. We are looking for the latter. If you have examples of the former, we encourage you to report them to https://bugs.swift.org.

12 Likes
2

Oops, I definitely shouldn’t be listed as the owner anymore! For now, I’ll just say that there’s currently no way for the importer to know what code belongs to you and what’s in someone else’s C library that you can’t change, and that’s pretty important for a diagnostic like this. So the opt-in mode is nice for running things one-off, but it’s not something we’d ever want to turn on by default. In order to do that, I’d suggest:

  • Providing a way to indicate which modules or headers are important. The simplest answer is “just the underlying module for a mixed-source framework”, but that doesn’t cover SwiftPM, which doesn’t allow mixed-source frameworks, or bridging headers for apps, which let you mix your own headers with arbitrary other headers that might not belong to you.

  • Providing a way to silence the warning on a per-decl basis in your own code.

But it’s not necessary to do those things just to have this available in the compiler. I just want to bring them up early enough that we don’t turn this on and then get dozens of bug reports about unsilenceable warnings.

4 Likes
3

Improvements in this area are very welcome; it's been a frequent pain point for us at Google since our Objective-C code is broken up into many small modules and they often use forward declarations instead of #imports to speed up compile time.

How about separating the warnings from the improved error diagnostics? I agree with @jrose that having the warnings visible on every import would be too much noise, but surfacing that same information (as you've done) in a note associated with the error if someone tries to use the decl is still a huge improvement over the current situation where the decl is just silently dropped.

As an alternative for the warnings, a good place where I think the information could be surfaced would be in the SourceKit interface generation request for the Clang module. When you render the Swift interface for the C/Objective-C decls, you could still include the ones that were unavailable, with the appropriate @available attributes that include the message why, so they would be visible to anyone viewing that in Xcode or some other tool that generates it for the module.

6 Likes
4

Lovely to see work in this area and agree with feedback above.

A more minor point is that this note is (to me) actively misleading in this context: "explicitly marked unavailable" strongly suggests intentionality on the part of an author. However, what's failed to import has been marked unavailable by an automated tool--indeed, the point of the improved diagnostics is to surface something that no human might have intended.

5 Likes
5

Thanks for your feedback. We might have focus a little bit on first party code, but we will definitely keep system and third party code in mind while we keep working on this. We imagine that the modules marked system should be easy to opt-out automatically from all these warnings, but we will keep an eye. We might need to keep this as strictly opt-in, which while not ideal, it is for sure less noisy.

We will have to look at this. We are still learning how the Clang Importer works, and figuring out the best way to approach this. Investigating how to attach those notes to the unavailability warning might be a good investigation path. Thanks!

We weren't planning in looking at SourceKit, but we did not know that it had that functionality. This can be something interesting for the future. Thanks for mentioning it.

We can look into the messaging and see if it can be improved. We are using the same code as it was already used by the functions that have variable arguments and others (https://github.com/apple/swift/blob/c2fd49cebb37a7851c71f27d8aedbea136bf0614/lib/ClangImporter/ImportDecl.cpp#L9831-L9838) which attaches an unavailable attribute to the node (I think the message comes from https://github.com/apple/swift/blob/e845af8fc2d8056b6ee5f296bd028396139b44f5/include/swift/AST/DiagnosticsSema.def#L5315-L5317).

6

Hey @NuriAmari! Sorry for the slow reply. I’m super excited about this work, and I think it will be a huge quality of life improvement for programmers using Clang inteorp.

First of all, I think you got a lot of things right here: putting this behind a flag is a good idea. Lot’s of APIs don’t import well and it will be quite noisy. This flag will probably be used by us (people working on the clang importer) more than anything else.

Second, I really like the idea of telling people why Swift can’t find the name they’re trying to use. It will help a lot of people debug compiler failures if they understand that it’s failing because we couldn’t import their API for whatever reason.

I do have some suggestions: first, it would be great to get warnings about why something couldn’t be imported. If you agree with my above statement that we’re are going to be some of the biggest users of the warnings flag, then it makes sense to target the warnings at us and our workflows. Rather than having a warning like “type X is unavailable in Swift” it would be nice to say something like “failed to import non-copyable type.” Or “failed to import type with no destructor.” Because the other “kind of signal” is targeted at Swift users, this comment doesn’t apply to that path as much (though, users may appreciate some extra information about why we couldn’t import their API).

If we had this extra information, it would also be super helpful for gathering statistics. This is something I hacked together a while back, but it really helps to know what kinds of things we see a lot and generally have a hard time importing (I found that non-copyable types were some of the most common, for the Swift compiler headers).

This comment also applies to forward declared classes. I think it would be really great to add some type aliases to OpaquePointer and import forward declared classes as those type aliases instead. For example, we might import a function that uses a pointer to a forward declared class as func test(c: ForwardDeclaredPtr) or a pointer to a move only class as func test(m: PtrToMoveOnly) (we can workshop the names a bit). Also (side-note), we should probably only import pointers to forward declared classes as OpaquePointer s and not pointers to any-class-we-fail-to-import.

Now on to the actual implementation... I’ll try to review that PR, but it might take some time, sorry.

First: I’m not sure the wrapper you introduced is necessary. And I’m worried the diagnostics will often be dropped on the floor. There are many places that importDecl is called, and it would introduce some complexity into the compiler to update them all to handle the diagnostics they find (and I’m worried many callers simply wouldn’t handle the diagnostics). Why not just emit the diagnostics right when we fail for some reason? This would also make it easier to specify why we failed. And, because these diagnostics are targeted at us, it’s OK if they’re not perfect.

Second: I’m also worried the stub generation isn’t the ideal solution here. Using @available attributes sort of box us in (we can’t generate notes, etc.). Also, we have to add a bunch of logic to create dummy stubs. And, it’s possible these dummy stubs would cause problems in the future, for example, if we’re trying to query information about a type. (I’m not too worried about this, but it is also a bit slower.)

I think a much simpler and better solution would be to hook into the member lookup information. I’m happy to help you with this (either over the forums, a video call, or some other medium). But, essentially, I think it would be pretty easy to add a condition in ClangDirectLookupRequest::evaluate , DirectLookupRequest::evaluate , or a request that makes more sense that asked “did we fail to import this name?” Because these are triggered lazily, we know that a user specifically asked us for this name. And, if we find a clang decl, we know that the name can be looked up in C++. If that’s the case, just emit a diagnostic. In fact, if you wanted to have a more fine grained diagnostic (and use the diagnostics from the “first flow”) you could even do something like this: before we try to import a clang decl, enable the diagnostics/warning in the importer, the diagnostics will fire when we fail to import the decl, then you can emit a proper error when we get nullptr back. This is best of both worlds: the user got an error saying something like “cannot use X that we failed to import” and they know why we failed to import it :)

Overall, I’m really excited about this! Feel free to ping me offline if you want to discuss it more.

2 Likes
7

Hi @zoecarver, thanks so much for your input, you've certainly raised a few points I hadn't considered. Some answers to your questions and general thoughts to follow.

Firstly, on the intended target of these diagnostics. When we set out to start this project, the target was very much regular Swift users, not compiler engineers. That being said, I'm very open to changing the style of the diagnostic messages, perhaps we could even have a verbose version of the flag that produces some more detailed information. This way we could cater better to both audiences.

Next, about the necessity of the wrappers. I wholeheartedly agree that there is significant risk to the diagnostics being dropped. As you can imagine, the wrapper carries diagnostics information "up the tree" to a point where we have enough context to act upon it. Admittedly, usually this means until we are ready to produce a stub. However, it also allows for more helpful diagnostic messages, especially when it comes to types.

Now about the stubs. I certainly agree that using the @available attribute limits us and I've already started pushing against this limitation. That being said, the stubs are rather important for a few reasons:

  • Ending lookups early
    • Lookups come in the form of repeated increasingly broad searches, often ending in a search that asks "import every declaration in this module" (to support typo fix suggestion).
    • Each of these searches can come with numerous ClangImporter invocations resulting in a great deal of diagnostic noise, including repetition.
    • The stubs stop the search early (which will fail eventually anyways) and eliminate this noise.
  • Reliably delivering relevant diagnostic information
    • The DiagnosticEngine has a limitation where when invoked in a reentrant manner, all diagnostics beyond the first are suppressed.
    • The ClangImporter and the lookup system both have caches and diagnostics are not produced if an import / lookup hits a cache. Also caching the diagnostics involves the wrapper situation x100.
    • These two factors mean, especially if you want relevant, usage triggered diagnostics, stubs are the only reliable mechanism we've found for delivering diagnostics.
  • Helping the user fix their issue
    • The stub with source location is much more user friendly than a lookup failure based alternative

This in mind, the stubs and by extension the wrapper, really help us combat situations where the ClangImporter / name resolution system do not play well with diagnostics, rather than providing great value themselves. As you've mentioned, a system allowing us to attach notes to a declaration marked unavailable would be ideal for us. The change being behind a flag, I am not too worried about slow down or type information queries being affected.

Regarding the forward declarations, what you propose sounds reasonable to me, though I am not versed with the representation of pointers to incomplete types using OpaquePointer. Thus far I focused mostly on larger constructs (methods) that are dropped as a result of referencing incomplete types.

Finally, on your suggestion of using the lazily triggered lookup requests. I think there is certainly some value here. If we find a way to fix the problems stubs help mitigate, I think this would be ideal. Stubs have some value on their own, but they also cause problems so I would really like an approach like this.

Overall, thank you again for your response. We are still learning a great deal about the ClangImporter's quirks and this has been most helpful. If you know of solutions to the issues I've raised or want to discuss more, feel free to reach out however is convenient. These are all my thoughts for now, but if you don't mind you might hear from me later once I've had some time to mull things over.

8

Thanks for the quick reply!

If we diagnose an error, this lookup should stop. If it doesn't that's a bug that is hopefully easy to fix.

I don't really see how this would be different for diagnostics triggered from @available..

We probably shouldn't diagnose something if it's cached. We should diagnose it the first time, then if we cache the result (which we currently don't do when we fail to import something) it won't be diagnosed again. I'm not sure I follow the last sentence.

This does not make sense to me. With a stub, you only get the source location from the "imported module." It will look like a Swift API which will likely confuse the user. With a custom diagnostic, it can be whatever you want. We could make it error at the call side, then have a note that shows where the API was defined in C++ and say why we couldn't import it (in the future, maybe even noting the deleted destructor, for example).

I agree. And hopefully my above comments show that we can fix the problems you're running into. I'm happy to show you exactly how I think we can implement this. Take a look at the two requests I mentioned above, I think that's where you want to hook into.

Thanks again for all your work on this!

1 Like
9

Can we do better for incomplete types than just OpaquePointer or type-aliases of it? One of the most frustrating things about Swift's current C interop is that opaquifying (or vice-versa) C structs can be mostly non-source-breaking change in the C API, if the API vends pointers to the struct and the client doesn't access the fields directly, but when imported into Swift, pointers to the incomplete type are erased down to OpaquePointer. Swift clients have to update their code, and worse, all type safety is lost because pointers to different incomplete structs are indistinguishable. (Update BoringSSL to 25773430c07075a368416c3646fa4b07daf4968a by Lukasa · Pull Request #315 · apple/swift-nio-ssl · GitHub is a recent example of having to modify Swift code to account for this change in C.)

I've always wondered if it would be possible to import incomplete types like this in Swift as caseless enums (or a memberless struct with a private initializer) instead, with the actual type name, and have pointers to them just be Unsafe(Mutable)Pointer<X>. It seems like that would help to make opaquifying C structs still type-preserving in Swift.

For Objective-C classes it may not be so easy, since the pointer to the class is imported as just a reference rather than an Unsafe(Mutable)Pointer. And there are probably many other edge cases I'm not considering here... but it would be great to move in that direction if we can.

3 Likes
10

This is a really great idea. If I have time I'll try to put up a patch/post about this. It might be best to hide it behind an annotation, though, so we don't accidentally make it look like we can import types that we can't. (Though, with proper diagnostics, it might be OK because we could get an error like "cannot access fields of forward declared struct".) Maybe this fit's in with foreign reference types somehow...

IIRC we don't import pointers to forward declared Objective-C classes anyway :)

1 Like
11

I think that would be fine, though? I don't think the lack of fields or an inability to instantiate it would be any more surprising to someone writing Swift than it would be to someone writing C using the original declaration, but we maybe we could synthesize documentation or a Swift attribute that would show up in the Swift synthesized source interface to denote such types.

My concern about using an explicit Clang annotation (if that's what you're referring to) to trigger this behavior is that it effectively requires the author of the C API to make it Swift-aware. That's fine for some codebases, but for the majority of low-level or external C libraries where this incomplete type erasure is a problem, it's not feasible to modify the original APIs.

This is one of those things that I would love to see land as a "Swift 6 breaking change".

True! I was thinking whether it would be possible to incomplete Obj-C classes as a possible alternative to dropping the referencing declarations entirely or making them inaccessible. The catch there is that, since the Obj-C pointer bridges directly to the unadorned Swift type, would there be any issues with such a class type existing in Swift that you can't do interesting things with (subclass, instantiate, access members), but still have it possible to call an imported Obj-C API that returns one of these pointers/references and then pass it to another Obj-C API that takes the pointers/reference...

1 Like
12

I want to clarify one point about the mention of "forward declarations" in the first post that might have been misinterpreted.

We are focusing in Objective-C @class forward declarations at the moment, because those are the ones that our code base uses, and those are the ones that are generating more problems for us.

We weren't trying to improve about C forward declared types that get transformed into OpaquePointer. Personally, I am aware of the limitations, and I would like to see work on that area, but it is not our current focus.

As a side note: Objective-C forward declarations are supported and imported in several scenarios (they are actually necessary for mixed language modules, but they work outside those too). There's one scenario (which is common place in our code base), which is not imported and causes a cascade of non-imported names which is the scenario that we are trying to diagnose.

1 Like
13

Hmm, I certainly hope it is an easy to fix bug. If there is a mechanism by which the ClangImporter communicates results back to lookup calls besides the nullness of the returned Swift AST node pointer, I would appreciate any pointers to it. Specifically here https://github.com/apple/swift/blob/main/lib/Sema/PreCheckExpr.cpp#L420 I've been experiencing the lookup not terminate early. If there is any easy way to determine if an UnqualifiedLookup fails with or without a diagnosis, I haven't found it yet. If we can solve this problem, our reliance on stubs is reduced greatly.

Fair point, I suppose these diagnostics could also be discarded, though I know this is happens within the ClangImporter, I suspect it is also the case outside, but I'm not sure:

Either way, I think the @available case is more rare and less impactful even if it does occur. Let me elaborate. ClangImporter diagnostics are discarded when a diagnostic (ex. pretty printing of class) points to an imported declaration. Any diagnostic produced during the import of said declaration is dropped. To make matters worse, sometimes loading is done in batches (ex. all methods on an Obj-C class are loaded at once). This means more dropped diagnostics. I would say there are comparatively fewer cases in which a diagnostic points to a node marked unavailable, which is then dropped.

However, putting frequency aside, the greater issue is that to my understanding, a diagnostic attached to an @available attribute will be displayed later if the user references the marked entity. I don't think the same guarantee exists for ClangImporter diagnostics, mostly due to caching. If the import succeeded, any warning diagnostics might be lost forever, as, if I understand correctly, the ClangImporter caches successful imports. In my case, when I return a stub to represent a failed import, this stub can even be cached outside the ClangImporter entirely. Then when I go to use the stub, the name lookup is already cached and the diagnostics are lost. Obviously, this is an issue with stubs, but stubs to replace unimported entities are not really something new we are introducing (ex. stubs are used to represent unsupported variadic C functions). I hope that helps explain where I'm coming from.

Yes, but like I've described sometimes we get unlucky and the DiagnosticEngine will not let us display the diagnosis the first time. Minor point, but sometimes the first time is during a lookup for something relatively unrelated. Then later when a more directly related lookup is performed, the user loses out on the then relevant diagnostics. Not an easy problem to solve, but hopefully displaying the diagnostic at least once is achievable. By the last sentence I meant, I'm not sure how to pass diagnostic information up to where it can be cached without wrapper structures around lookup results.

I think I was somewhat misguided in making this point, but you've since convinced me. Thanks :).

Sure, some more detail would be wonderful. I have actually poked around with this some of this request code already (this is uses the lookup cache I've been having issues with). I guess my question here is the same as for an UnqualifiedRequest: Is there a way to place a failed lookup in one of the following categories:

  1. No results found
  2. ClangDecl found but couldn't be imported for a diagnosed reason
  3. ClangDecl found but import failure not diagnosed

Of course if we are scenario 2, we also would like to know the diagnosed reason. Anyways, thanks again for your input, and I hope I've been adequately coherent in my response. I'll contact you maybe we can find some time to discuss things.

14

OpaquePointer was previously discussed (circa Swift 4.2):

1 Like
15

Thanks for the link! It's a shame that didn't go further at the time; I'd love to see it revisited if we're considering beneficial source-breaking changes for a Swift 6 mode, especially as Swift wants to position itself as interfacing well with C (and eventually C++).

3 Likes
16

Hi everyone, based on all the feedback received (thank you!) we have revamped our approach and are rebuilding it here: [WIP] Improve ClangImporter Diagnostics by NuriAmari · Pull Request #39923 · apple/swift · GitHub. Overall the new approach uses the following ideas to avoid reliance on stubs:

  • Use of a pending error note queue in place of wrapping return types to pass diagnostic information up the tree
  • Taking advantage of @zoecarver's work on making the importer lazier to only produce diagnostics for Clang nodes directly referenced by the user
  • Use of a diagnostic target (a ClangDecl for which we are interested in diagnostics for) to help suppress diagnostics about unrelated nodes
  • Use of a set of visited of diagnostic targets to reduce diagnostic repetition
6 Likes
17

Hi everyone,

A few days ago a patch was landed to implement an initial set of improved diagnostics. A big thank you to @zoecarver, @drodriguez and @plotfi for helping me along the way.

Flags:

The new diagnostics are hidden behind two frontend flags. The first flag -enable-experimental-clang-importer-diagnostics enables lazily triggered diagnostics (that is diagnostics are only produced for Clang nodes that are referenced from Swift, but fail to import). This flag is suitable for day to day usage. The second flag enable-experimental-eager-clang-module-diagnostics forces the compiler to import all visible declarations from each loaded Clang module and produce diagnostics for any failures. This flag is far too noisy for day to day usage, but useful for debugging the ClangImporter or checking the importability of modules.

Diagnostics:

An exhaustive list of the current diagnostics can be found in the test/ClangImporter/experimental_diagnostics* tests such as this one. Here I'll present in my opinion the most useful category of diagnostic.

Incomplete Types:

The ClangImporter drops declarations that refer to types that failed to import. The type can fail to import for numerous reasons. In the default compiler configuration, incomplete types are considered a failure. Here we have a header that contains various declarations referring to incomplete types.

#include <Foundation.h>

@class ForwardDeclaredInterface;
@protocol ForwardDeclaredProtocol;

@interface Bar : NSObject
@property id<ForwardDeclaredProtocol> propertyUsingAForwardDeclaredProtocol;
@property ForwardDeclaredInterface* propertyUsingAForwardDeclaredInterface;
- (NSObject<ForwardDeclaredProtocol> *) methodReturningForwardDeclaredProtocol;
- (ForwardDeclaredInterface *) methodReturningForwardDeclaredInterface;
- (int)methodTakingAForwardDeclaredProtocolAsAParameter:(id<ForwardDeclaredProtocol>)param1;
- (int)methodTakingAForwardDeclaredInterfaceAsAParameter:(ForwardDeclaredInterface *)param1 andAnother:(ForwardDeclaredInterface *)param2;
@end

ForwardDeclaredInterface* CFunctionReturningAForwardDeclaredInterface();
void CFunctionTakingAForwardDeclaredInterfaceAsAParameter(ForwardDeclaredInterface* param1);

NSObject<ForwardDeclaredProtocol> *CFunctionReturningAForwardDeclaredProtocol();
void CFunctionTakingAForwardDeclaredProtocolAsAParameter(id<ForwardDeclaredProtocol> param1);

Referencing any one of these declarations from Swift will fail.

import IncompleteTypes

let bar = Bar()
_ = bar.methodReturningForwardDeclaredInterface()

Previously only the last diagnostic was produced, now there are warnings and notes to provide context:

.../IncompleteTypes.h:10:1: warning: method 'methodReturningForwardDeclaredInterface' not imported
- (ForwardDeclaredInterface *) methodReturningForwardDeclaredInterface;
^
.../IncompleteTypes.h:10:1: note: return type not imported
- (ForwardDeclaredInterface *) methodReturningForwardDeclaredInterface;
^
.../IncompleteTypes.h:10:1: note: interface 'ForwardDeclaredInterface' is incomplete
- (ForwardDeclaredInterface *) methodReturningForwardDeclaredInterface;
^
.../IncompleteTypes.h:3:1: note: interface 'ForwardDeclaredInterface' forward declared here
@class ForwardDeclaredInterface;
^
.../experimental_diagnostics_incomplete_types.swift:8:9: error: value of type 'Bar' has no member 'methodReturningForwardDeclaredInterface'
_ = bar.methodReturningForwardDeclaredInterface()
    ~~~ ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

This behaviour is not limited to incomplete types. For example, unsupported built-in C types such as _Complex also trigger such an error. Lastly, there are also diagnostics for C struct declarations and unsupported macros not shown here.

Adding New Diagnostics:

Adding new diagnostics is fairly simple. Make a call to addImportDiagnostic as shown here, providing the ClangNode to attach the diagnostic to, the Diagnostic object itself and an optional source location. Lastly, make sure that the type of ClangNode you've attached the diagnostics to is visited by the DiagnosticWalker. For most ClangNodes, the existing visit methods should be sufficient. I'm happy to explain in more detail, but in short the DiagnosticWalker exists to decouple diagnostic emission behaviour from importing behaviour.

Conclusion

I hope these diagnostics are useful, or at the very least provide a mechansim for more useful future diagnostics within the ClangImporter. If anyone has any questions, feedback or suggestions, I'm happy to field them.

3 Likes