Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for manually written code injection #10

Open
anderslanglands opened this issue Nov 28, 2020 · 5 comments
Open

Add support for manually written code injection #10

anderslanglands opened this issue Nov 28, 2020 · 5 comments
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@anderslanglands
Copy link
Contributor

We need a way to specify code to be directly injected into the bindings, primarily to manually override the binding of specfiic functions, but also perhaps for arbitrary extra API to be provided.

The easiest way to do this would be to figure out a naming convention such that the user could provide e.g. a c-imageio-manual.cpp whose contents would get injected into c-imageio.cpp. Another alternative would be adding a command-line option to cppmm to specify a directory where manual bindings are to be found, and copying the contents of any files with the same name as the binding files into the binding TU, or perhaps adding the files directly to the generated project.

@anderslanglands anderslanglands added enhancement New feature or request good first issue Good for newcomers labels Nov 28, 2020
@scott-wilson
Copy link
Member

Is this suppose to replace the namespace cppmm_manual {}?

Here's a few other options that I can think of for getting the files for injection.

  • Command line option like --manual-injection-suffix=-manual would match c-imageio.cpp with c-imageio-manual.cpp.
  • We could match the ext .cppm. So c-imageio.cpp matches c-imageio.cppm.

I personally like the cppm option (or something like it), because it keeps all the files together and programming it should be pretty easy (orig_file_name + "m"). We could do .in instead, so c-imageio.cpp matches with c-imageio.cpp.in.

I can take this issue.

@anderslanglands
Copy link
Contributor Author

Hi Scott, yes exactly. The namespace idea doesn't really work because you want everything in the bind file to compile without errors, and any manual definitions will always error because you haven't defined the types they're using yet!

It would actually technically work, i.e. clang will actually generate the AST correctly for the rest of the file, so the resulting bindings should be good, but the idea of the tool spitting out a boat-load of errors by default doesn't sit right with me.

Perhaps that idea could be salvaged if there were a way of making clang ignore that entire block, but I don't know one other than commenting or #define-ing it out.

As for your suggestions, yeah I think both are good. I think I would personally lean towards name-manual.cpp or something shorter like name-m.cpp for two reasons:

  1. name.cpp and name.cppm are visually close enough to be annoying when Ctrl-Ping in your editor
  2. Using an extension other than .cpp risks being annoying for anyone whose editor relies on .cpp extension for correct syntax highlighting (including github maybe? havent' checked).

but if you feel strongly the other way I'm fine with that.

@scott-wilson
Copy link
Member

That's a fair point on the name-manual.cpp vs name.cpp.in. I'm not too bothered either way, as long as we eventually have some documentation to make this clear.

So, here's what I'm hoping to do.

Say we have the "auto" file:

my_bind.cpp

namespace cppmm_bind {
bool do_thing();
}

We'd expect the result to look something like this:

my_wrapper.h

bool do_thing();

my_wrapper.cpp

extern "C" {
bool do_thing() {
    // Does thing here
}
}

What I'd want for the "manual" wrapper is to be able to do something like this:

Given my_bind_manual.cpp

namespace cppmm_bind {
int do_custom_thing() {
    // Insert some custom logic here
    return 42;
}

void mytype_say_hello(const mytype* self) {
    self->say_hello();
}
}

The wrappers would look something like this (including from the "auto" wrapper):

my_wrapper.h

bool do_thing();

int do_custom_thing();

void mytype_say_hello(const mytype* self);

my_wrapper.cpp

bool do_thing() {
    // Does thing here
}

int do_custom_thing() {
    // Insert some custom logic here
    return 42;
}

void mytype_say_hello(const mytype* self) {
    self->say_hello();
}

Does this seem reasonable? Or is there something that I'm clearly missing?

@anderslanglands
Copy link
Contributor Author

Hi Scott, yeah I think that's essentially it!

@scott-wilson scott-wilson removed their assignment Apr 24, 2021
@scott-wilson
Copy link
Member

Unassigned myself, since I don't think I'll be able to add this yet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants