Skip to content

Implement functions for C++ interface#239

Open
frostedoyster wants to merge 4 commits into
metatomic-corefrom
metatomic-cpp
Open

Implement functions for C++ interface#239
frostedoyster wants to merge 4 commits into
metatomic-corefrom
metatomic-cpp

Conversation

@frostedoyster
Copy link
Copy Markdown
Contributor

@frostedoyster frostedoyster commented May 28, 2026

This PR implements the metatomic C++ interface

Contributor (creator of pull-request) checklist

  • Tests updated (for new features and bugfixes)?
  • Documentation updated (for new features)?
  • Issue referenced (for PRs that solve an issue)?

Reviewer checklist

  • CHANGELOG updated with public API or any other important changes?

📚 Download documentation for this pull-request

⚙️ Download Python wheels for this pull-request (you can install these with pip)

@frostedoyster frostedoyster requested a review from Luthaf May 28, 2026 14:40
Copy link
Copy Markdown
Member

@Luthaf Luthaf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will also need classes for PairListOptions, ModelMetadata, and Quantity that do the conversion to and from JSON

Comment on lines +78 to +81
// The C plugin registry keeps this pointer; allocate stable process-lifetime storage.
auto* name_storage = new char[name.size() + 1];
std::memcpy(name_storage, name.c_str(), name.size() + 1);
details::PluginRegistration<PluginT>::name = name_storage;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Rust code makes a copy of the name, there is no need to keep it alive

Comment on lines +69 to +70
template<typename PluginT>
void register_plugin(PluginT& plugin) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we need this as a public API versus something hidden inside the upcoming MTA_REGISTER_PLUGIN macro, but we can keep it for now

Comment on lines +91 to +94
/// Register a raw C plugin.
inline void register_plugin(mta_plugin_t plugin) {
mta_register_plugin(plugin);
}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not very useful

inline Model load_model(
const std::string& plugin_name,
const std::string& load_from,
const std::vector<KeyValuePair>& options = {}
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you rebase on top of #237 and update the API accordingly?

namespace metatomic {

/// RAII wrapper around a `mta_model_t`.
class Model final {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will also need a abstract base class to define a model in C++

@frostedoyster frostedoyster force-pushed the metatomic-cpp branch 2 times, most recently from 89c24c1 to d87def3 Compare May 29, 2026 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants