Model driven api

[Michael-F-Bryan]

This is an initial implementation of #804 based on the ideas I was trying in Michael-F-Bryan/fornjot-plugins. The discussion in #804 (comment) contains a more in-depth explanation of how it works.

[hannobraun]

Thank you, @Michael-F-Bryan, this is great! Looking forward to merging this!

I didn't go through this pull request line-by-line. This is a lot of code, and I figure it's more efficient to immerse myself in it over the next weeks and months of working with it, instead of trying to understand every detail right now. The CI build passes, so I'm confident it's working as intended, or close enough.

I did look at every commit though, and do have some comments:

• I think it's a bit unfortunate that within the fj crate, the code for defining models (everything that was there before this PR) is intermixed with the new code. I think it would be better to have the new code in a dedicated module, which is clearly documented as "don't worry about this, this is taken care of for you (by the proc macro)".
• If that were the case, then the abi module could be un-#[doc(hidden)]. I've worked with crates in the past that are hiding internal implementation details from the API reference, and I found this is often confusing, and just makes it more likely that I have to dive into the code.
• What's up with those todo!s that are still left in the code? Where those missed, or is that stuff that can be filled in later? (I mean, CI passes, so presumably the latter.)
• All that hand-written FFI code is unfortunate, but given that we'll most likely switch to WebAssembly and WIT in the future, it's fine. If that code turns into too much of a maintenance issue, we can accelerate the transition to WASM/WIT, by publishing our own fork of wit-bindgen to crates.io, as I suggested as a possibility in Switch to model-driven model system #804.

All of those comments are just my observations on where I see potential for improvement, or where I could see things go. None of them need to be taken care of in this pull request, and in fact I'd prefer if we could get this pull request merged quickly and take care of anything else later.

However, there is one thing that I see as a blocker: With this change, we're losing the convenient #[fj::model] syntax, and I'd rather keep that. That syntax is mentioned in the README and past release announcement, and we'd have to at least adapt the README to prevent false advertising.

I think it would be better to port the macro to the new infrastructure instead before merging this PR. What do you think?

[Michael-F-Bryan]

@hannobraun can you have another look at this? I've updated the #[fj::model] macro to generate the new code and switched the models (star, cubiod, etc.) back to their original implementations. From an end user's perspective, they shouldn't notice the difference other than the fact that you can optionally return an error from your model function now.

I think it's a bit unfortunate that within the fj crate, the code for defining models (everything that was there before this PR) is intermixed with the new code. I think it would be better to have the new code in a dedicated module, which is clearly documented as "don't worry about this, this is taken care of for you (by the proc macro)".

Agreed. The main barrier to splitting them is that we have a dependency cycle.

• We want to move the register_model!() macro to some hypothetical fj-models crate and re-export it under fj, however
• The register_model!() macro references Host, and
• the Host trait (via the Model trait) references Shape

What's up with those todo!s that are still left in the code? Where those missed, or is that stuff that can be filled in later? (I mean, CI passes, so presumably the latter.)

Oops! I forgot to wire up converting the FFI-safe version of Metadata to the canonical representation. The host doesn't access metadata at the moment, so the todo!() was never hit.

All that hand-written FFI code is unfortunate, but given that we'll most likely switch to WebAssembly and WIT in the future, it's fine. If that code turns into too much of a maintenance issue, we can accelerate the transition to WASM/WIT, by publishing our own fork of wit-bindgen to crates.io, as I suggested as a possibility in #804.

Yeah, it's not great.

In theory, we could use #[derive(StableAbi)] derive from the stable_abi crate to generate stable versions of any type that gets passed across the FFI boundary. It also takes care of the vtables/trait object stuff I was writing by hand. It wasn't until after I wrote all that FFI code that I remembered the stable_abi crate 😅

Sorry for the delay, I spent most of last week out of the city and away from a computer.

[hannobraun]

Thank you, @Michael-F-Bryan! I'll take another look later today.

[hannobraun]

Thanks again, @Michael-F-Bryan! Looks good!

I'm going to merge this PR now, and will address your comments in a separate comment.

[hannobraun]

I think it's a bit unfortunate that within the fj crate, the code for defining models (everything that was there before this PR) is intermixed with the new code. I think it would be better to have the new code in a dedicated module, which is clearly documented as "don't worry about this, this is taken care of for you (by the proc macro)".

Agreed. The main barrier to splitting them is that we have a dependency cycle.

• We want to move the register_model!() macro to some hypothetical fj-models crate and re-export it under fj, however

• The register_model!() macro references Host, and

• the Host trait (via the Model trait) references Shape

But I was suggesting to collect the new code in a module within the fj crate. That should be possible, right?

All that hand-written FFI code is unfortunate, but given that we'll most likely switch to WebAssembly and WIT in the future, it's fine. If that code turns into too much of a maintenance issue, we can accelerate the transition to WASM/WIT, by publishing our own fork of wit-bindgen to crates.io, as I suggested as a possibility in #804.

Yeah, it's not great.

In theory, we could use #[derive(StableAbi)] derive from the stable_abi crate to generate stable versions of any type that gets passed across the FFI boundary. It also takes care of the vtables/trait object stuff I was writing by hand. It wasn't until after I wrote all that FFI code that I remembered the stable_abi crate 😅

I actually experimented with stable_abi a while ago, hoping I could use their Vec-equivalent. I remember hitting some problem, but don't recall the details.

In any case, the code exists now. If it turns into a problem before the WASM/WIT migration, we have options.

Sorry for the delay, I spent most of last week out of the city and away from a computer.

No problem! Thank you for taking the time to update this PR and get it merge-ready!