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

Compiler error in generated WASM/Protobuf #138

Open
guthriec opened this issue Apr 15, 2023 · 6 comments
Open

Compiler error in generated WASM/Protobuf #138

guthriec opened this issue Apr 15, 2023 · 6 comments

Comments

@guthriec
Copy link

Hi,

I have an interface that includes some protobuf types (e.g.

my_service = interface + c {
  get_my_proto_type(arg: i64): ProtoType;
}

Generating for WASM works fine, using a command like:

../../../djinni/src/run \
    --java-out java \
    --wasm-out wasm \
    --wasm-include-cpp-prefix ../cpp/ \
    --wasm-base-lib-include-prefix ../../../../djinni/support-lib/wasm/ \
    --ts-out ts \
    --ts-module core \
...

However, the generated code runs into compilation errors when I try to compile using Emscripten:
emcc -o test.html wasm/my_service.cpp -I../../my_dependency
yielding an error:

wasm/my_service.cpp:27:26: error: use of class template '::djinni::Protobuf' requires template arguments
        return ::djinni::Protobuf::fromCpp(r);
                         ^
wasm/../../../../djinni/support-lib/wasm/djinni_wasm.hpp:272:7: note: template is declared here
class Protobuf {
      ^
wasm/concern_service.cpp:30:60: error: use of class template '::djinni::Protobuf' requires template arguments
        return ::djinni::ExceptionHandlingTraits<::djinni::Protobuf>::handleNativeException(e);
                                                           ^
wasm/../../../../djinni/support-lib/wasm/djinni_wasm.hpp:272:7: note: template is declared here
class Protobuf {

I notice I did not set --wasm-include-prefix, nor do I really understand what it means -- am I missing something there? Or is this a bug?

@li-feng-sc
Copy link
Contributor

The codegen for a protobuf return value should be like:

return ::djinni::Protobuf<...>::fromCpp(r);

Looking at the code I don't see how it could generate Protobuf without the template arguments.

Could you provide a minimal case(just the .djinni file and the .yaml protobuf declaration file, djinni does not actually use the protogen during codegen) that reproduces this problem(Protobuf without template arguments) so that I could have a closer look and debug it if there is indeed a bug?

@guthriec
Copy link
Author

guthriec commented Apr 17, 2023

Edit: I updated the errors and the repo to reflect some issues with bad imports in a prior version of the error repo.

I uploaded a repo with a minimal repro:

https://github.com/guthriec/djinni-error-repro

I'm on an Apple Silicon MacBook, in case that's relevant.

Codegen steps:
Core/API/CoreAPIProto/proto_gen.sh
Core/API/djinni_gen.sh

Then from the Core directory, running bazel build wasm results in errors:

Core/API/wasm/concern_service.cpp:27:26: error: 'Protobuf' is not a class, namespace, or enumeration
        return ::djinni::Protobuf::fromCpp(r);
                         ^
external/djinni/support-lib/wasm/djinni_wasm.hpp:272:7: note: 'Protobuf' declared here
class Protobuf {
      ^
Core/API/wasm/concern_service.cpp:30:60: error: use of class template '::djinni::Protobuf' requires template arguments
        return ::djinni::ExceptionHandlingTraits<::djinni::Protobuf>::handleNativeException(e);
                                                           ^
external/djinni/support-lib/wasm/djinni_wasm.hpp:272:7: note: template is declared here
class Protobuf {
      ^
2 errors generated.

@li-feng-sc
Copy link
Contributor

Thanks! I will take a look.

@li-feng-sc
Copy link
Contributor

So your problem is that you did not provide a ts: section in your yaml file, this is why Djinni does not know how to generate the code. See an example here https://github.com/Snapchat/djinni/blob/master/test-suite/djinni/vendor/third-party/proto.yaml#L9

We can probably make this a bit less confusing by providing better error message than generating incorrect code.

@guthriec
Copy link
Author

Ah ok that seems plausible. I haven't had a chance to verify yet but thanks!

@guthriec
Copy link
Author

I did get back to checking and got my setup working. A couple of more things that I think might be worth calling out in the documentation:

(1) Djinni expects the protobuf JS to be generated by https://github.com/protobufjs/protobuf.js/ (there are some other projects out there, including an official Google project that's not well-maintained).
(2) The need to call registerProtobufLib on the generated WASM module.

I'll leave this open just in case you want to update the docs.

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

No branches or pull requests

2 participants