Introduce protocol-based structure for generating code

[madsodgaard]

This goal of this PR is to introduce a protocol Swift2JavaGenerator, which does all the code generation. This will allow us to provide multiple generation modes, such as FFM or JNI.

Most of the stuff from Swift2JavaTranslator+... has been moved into FFMSwift2JavaGenerator.

[madsodgaard]

@rintaro I could use a bit of your thoughts here. Currently, it seems like the visitor depends on translated types, specifically this function

swift-java/Sources/JExtractSwift/Swift2JavaTranslator+JavaTranslation.swift

Line 18 in 99fc99c

func translate(

used in places like

swift-java/Sources/JExtractSwift/Swift2JavaVisitor.swift

Line 230 in 99fc99c

translatedSignature = try translator.translate(swiftSignature: swiftSignature, as: .initializer)

This stuff seems FFM specific, so should really live in the FFM implementation of the protocol and the visitor should not depend on it. It seems like the property translatedSignature is only actually used in the translation phase, not the analysis phase. So my initial thought was to postpone this translation to the actual FFM phase and not during the visiting phase. I just wanted to confirm with you whether you think this is a good idea, or if you have any better ideas?

[madsodgaard]

It seems kind of weird that this protocol just has an "empty" function with no arguments. But, I did that to prevent having to pass AnalysisResult to a bunch of sub-functions in print... So instead the implementations can receive them as initializer arguments.

[rintaro]

It's not clear for me how this protocol will be used. The main flow can be just like:

  try translator.analyze()
  
  if shouldGenerateFFM {
    FFMSwift2JavaGenerator(analysis, ...).generate()
  }
  
  if shouldGenrateJNI {
    JNISwift2JavaGenerator(analysis, ...).generate()
  }

Could you explain why we'd want this protocol?

[ktoso]

Yeah that’s pretty equivalent. The protocol being just the generate method seemed nicer to me, and we’d swap out what we generate with but it’s equivalent to that if statement. Since there’s not much to share either approach is fine i think, modeling wise i like the protocol but we could do the if

[madsodgaard]

Perhaps this should just be the return value of analyze()?

[ktoso]

Yeah, I think that'll be nicer.

The translator still is stateful since it may keep names for resolution etc but it'll be nicer to return the result like that 👍

[rintaro]

I'm totally fine with delaying FFM translate after the "analysis" phase, which will mean the AnalysisResult (i.e. ImportedFunc) would only hold SwiftFunctionSignature at most, not TranslatedFunctionSignature, right?

There are a couple of reasons I didn't do that:

• Wasn't sure sharing the visitor would be the way to go. It's just 250 lines of code. JNI generator might want to create its own visitor with its own needs. We could still share SwiftSymbolTable and SwiftFunctionSignature.
• Wanted to keep the TranslatedFunctionSignature somewhere around. Although I don't think it's used multiple times at this point, it's possible that we want to use it multiple places in the generator/printer.

That being said, I now feel sharing analysis result is a good move. To resolve the 2nd bullet, probably FFMSwift2JavaGenerator can hold the cached TranslatedFunctionSignature result for each ImportedFunc, (just like the thunkNameRegistry) I.e.

var translatedSignatures: [ImportedFunc: TranslatedFunctionSignature]

[ktoso]

Cool, that’s the outcome I was hoping for here — when we delay the translated signature the analysis can remain detached of the output, and FFM and JNI can do their own translation/lowering based on that result 👍

[rintaro]

Okay, I am moving the FFM translation part to after analyze(). #252

@madsodgaard Sorry for the conflicts, but I believe this makes JNI additions easier.

[ktoso]

Thank you Rintaro! :)

[madsodgaard]

@rintaro @ktoso I think this is ready for a first pass. For now, I have just kept the protocol, but as @rintaro pointed out, we don't actually do any dynamic swapping it in that way, but I'll just keep it around for now.

Otherwise, most of the stuff is just moving the previous translator stuff into FFMSwift2JavaGenerator, as well as a new FFM folder. The tests still assume we always test FFM, but we can perhaps look at how to tackle more generalized ways of testing both JNI and FFM once we get further and actually start generating JNI.

[rintaro]

The code moving part looks fine to me. I'll defer the command-line option part to @ktoso as it will affect #248

(As for the test failures, the workflow still uses older compiler which doesn't support trailing commas in parametar clauses)

[ktoso]

The license header failure is about Sources/JExtractSwift/GenerationMode.swift and other files missing the header we have in other files, can you add these please? :)

[ktoso]

I merged the tools rename, I can do the conflict resolution or leave it you -- let's catch up on chat :)

[ktoso]

Good! Btw I think we should start consistently prefixing things with java or swift; We used to have a bit of a mix, we'll now start converging on everything being prefixed with swift/java depending on which bit it applies to (like moduleNameI think we need to make intoswiftModule` etc.

[ktoso]

--mode sounds good to me 👍

[rintaro]

Not sure why #258 doesn't work 🤔 Maybe rebasing required?

HEAD is now at 7b8d615 Merge 30b0ce0fdfe1e751d13ff5540cdb336ee94b1b37 into fbebbe4fff84e903976f6cb5b5647ab915bfa235

fbebbe4 is the main ref before #258 merged