Generate bridging header for Swift

[4brunu]

tl;dr: Introduced --objc-bridging-header argument that allows to generate bridging header Objective-C->Swift.

In order to use Objective-C(++) in Swift, Objective-C Bridging Header must be created. Such header consists of imports of all Objective-C headers that will be used in Swift code. For support of Objective-C++ (generated code in djinni is a case) also *.mm file must be created that will import Objective-C Bridging Header.

Due to the fact that such Bridging Header content consists of lines that depends on generated content by djinni, whole Bridging Header could be generated too. In this PR you will find changes that generate Objective-C Bridging Header (and associated *.mm file for Objective-C++ support).
👍 3

This PR is a continuation of #226.
I picked the work done by @michal-kowalczyk, fixed the error, and apply the code review changes requested by @artwyman.

I don't claim the credits of this work to me, all the credit's should go to @michal-kowalczyk.

[smarx]

Automated message from Dropbox CLA bot

@4brunu, it looks like you've already signed the Dropbox CLA. Thanks!

[artwyman]

Thanks for picking this up. Looks good overall. I'm no Swift expert, though I'll try to get someone else at Dropbox to have a look for comments on that angle.

High-level: See my comments on the example app, where I'd like to see useful examples of both ObjC and Swift. I'd also like to see some Swift usage added to the test suite.

[artwyman]

Given this is acting as sample code, I don't like that there's now no example in use of ObjC implementing a Djinni class. TXSTextboxListenerImpl.h is still around but not used.
Could you expand the example to demonstrate both? E.g. have two different impl types used at different times, or where one calls the other, or with a run-time condition to pick which one gets used (so that both codepaths are compiled, even if not run).

[4brunu]

I will try to do this next week.

[artwyman]

Thanks.

[4brunu]

It's done, I also updated the Android example to work with the new use case

[artwyman]

Would best-practice suggest also generating a .m file which includes this? I noticed that in the old diff.
I'm fine either way, though if the new arg causes 2 files to be generated, it should take a base name to which both extensions can be added, rather than taking the full name of one file and removing its extension to name another file.

[4brunu]

I remove the .m file on propose.
Since this is just a bridging header and the only function it's to import another files, I followed the same approach that Apple did.

[artwyman]

👍

[artwyman]

Is there a reason to treat this as an independent generator rather than a part of ObjC generation? From a code structure point of view I'm glad to have SwiftBridgingHeaderGenerator.scala be separate, but from a user point of view does it ever make sense to generate the header other than at the same time as generating ObjC? If the two were combined, with the bridging header being an optional output, then it would also make sense to have a single output directory, and not have to deal with the cases where the two output directories differ.

[4brunu]

Yes, it's because the generators work differently.
The ObjC generator writes the files one but one, and only start the next one, when the current is finished.
The SwiftBridgingHeaderGenerator capture the ObjC file names while they are being generated, and writes them to the BridgingHeader.
So the SwiftBridgingHeaderGenerator is writing the BridgingHeader since djinni start generating ObjC files, until it finishes.

[artwyman]

I'm not sure I follow. All the swift-bridging-header generation seems to happen all at once inside of SwiftBridgingHeaderGenerator.generate(). This long-lived writer would make sense if the bridging header was being output piecemeal as a side-effect of ObjC generation, similar to what's done with the outFileListWriter. With the current implementation, it seems like it could be handled entirely inside of SwiftBridgingHeaderGenerator.generate().

Regardless, my prior comment was less about the implementation than about the model from a user perspective. There's extra complexity if the objc and swift-bridging-header generators can be enabled separately, and given different output directories (the latter is why you need to create the directory below). I feel like it might be simpler for the user to see the swift bridging header as simply an optional output of the ObjC generator, which they simple enable optionally, and provide with a filename (not a path).

[4brunu]

Sorry, I miss read your statement and explained myself wrongly because I implemented this some time ago and I have little knowledge of the djinni source code.

Now instead of asking for the bridging header path --objc-swift-bridging-header-out "$temp_out/objc/TextSort-Bridging-Header.h" it only require the bridging header name --objc-swift-bridging-header "TextSort-Bridging-Header" and the file will be generated to the objc output directory.

[artwyman]

Thanks for making modifications to the example, but I'm not sure I really understand it, or that it's the best example. SortItems right now is a stateless object representing a sort algorithm, so it's weird for it to be involved in the reset operation, in which it's not doing anything other than calling right back to its listener, which is more of an executor than a listener. I also think it's a shame to add complexity to the Android example when it's only there as a demo of Swift.

I'd suggest keeping the new functionality limited to the iOS example. It might make sense to just have two different implementations of the same listener which do different things (log internally, vs. put up a toast?), with a toggle in the UI which switches between them. Then the Android example could remain entirely unaffected.

Note I also added @wkiefer to this review, as the Dropboxer with the best context on Swift usage. We're not using Djinni with Swift yet, but we're likely to do so eventually, so I'd like Will's thoughts to make sure this PR meets our needs when that happens.

[4brunu]

@artwyman I simplified the example has you suggested, could you please take a look a it now to see if it's ok?

[artwyman]

Thanks for the simplified example, I think it'll be much easier for new users to understand.
I'm going to wait for @wkiefer to have a chance to take a look before merging.

[wkiefer]

Apologies for the delay - looks great. I left minor stylistic comments for the Swift code to be a bit more idiomatic.

Thanks!

[wkiefer]

I'd recommend removing the top-of-file boilerplate.

[wkiefer]

trailing underscores are not idiomatic in Swift, i'd recommend renaming to textView (and you'd do self.textView = textView on line 16 below

[wkiefer]

The @objc isn't needed when you subclass an Objective-C class (like NSObject). Could remove.

Would also suggest adding final

[wkiefer]

Could also remove @objc here because it's implied by the NSObject subclass

[wkiefer]

nit: typically no space between init and (

[wkiefer]

you could add

    @available(*, unavailable)
    init() {
        fatalError("Unsupported")
    }

to be explicit the default NSObject initializer is not available.

[4brunu]

@wkiefer it's done :)