dropbox / djinni

A tool for generating cross-language type declarations and interface bindings.
Apache License 2.0
2.88k stars 488 forks source link

fixed `isInterface` function, when given Interface is MExtern (issue 401) #420

Closed ctin closed 5 years ago

ctin commented 5 years ago

https://github.com/dropbox/djinni/issues/401

xianwen commented 5 years ago

Hi, @ctin: I noticed you haven't signed the CLA yet, could you please sign it here: https://opensource.dropbox.com/cla/ Thanks a lot!

ctin commented 5 years ago

Hi, @xianwen Done.

xianwen commented 5 years ago

Hi ctin:

Generally LGTM, and passes all tests. Could you please add a test case in the example test suite, so that we can verify the fix and prevent from regression in the future?

Thanks!

paulocoutinhox commented 5 years ago

Hi @ctin,

Can you add a test to merge it?

Thanks.

ctin commented 5 years ago

To correctly add test for my merge request I need: 1) create yaml file with an interface:

extern_test_interface = interface +c +j +o {
}

2) create djinni file with a record, which are using that interface:

@extern "extern_test_interface"

test_extern_interface_record = record {
extern_test_interface = optional<extern_test_interface>;
}

in current master record above will have line:

std::experimental::optional<std::shared_ptr<::extern_test_interface>> extern_test_interface;

but with my fix will be generated line:

std::shared_ptr<::extern_test_interface> extern_test_interface;

I have next questions: 1) where I can place my yaml file? 2) how I can check that correct type is generated? 3) how to add this check to java and objc tests?

ctin commented 5 years ago

@xianwen please review