Rantanen / intercom

Object based cross-language FFI for Rust
MIT License
63 stars 7 forks source link

Implemented wrapper class for safe raw interface management #44

Closed Fluxie closed 6 years ago

Fluxie commented 6 years ago

The RawInterface class manages the reference counts of the interfaces and calls Release for the interface when the wrapper is destroyed. The interface of the RawInterface class has been mimicked from std::unique_ptr.

Rantanen commented 6 years ago

The AppVeyor build fails since the wrappers are only used for the GCC build.

Perhaps we should do the following:

How does this sound?

Rantanen commented 6 years ago

I added the allocator tests to the test_lib.cpp/test_lib.h, which resulted in conflicts with this branch.

So as a result, I removed those two files from git. They are now generated through the cmake using the test/cpp-raw/CMakeLists.txt. There is also dedicated test-infra under intercom/tests/generators that can be used for tracking the changes in the generators. I believe that was the main reason these two files were in git instead of being generated?

The current test cases there are rather basic and not really that useful, but they should be rather easy to expand. Just modify the test_lib.rs or make a new one, such as collection_tests.rs. You can force update the target files (.h, .cpp, .idl, .manifest) by running the cargo test --all-features with UPDATE_XYZ_TESTS=1 environment variable. This is included as part of the failure messages when cargo test --all-features fails.

Fluxie commented 6 years ago

The AppVeyor build fails since the wrappers are only used for the GCC build.

I don't have en environment for testing stuff on Windows platform at the moment. I thought that as I didn't include the file anywhere, it wouldn't be included in the tests.

The wrapper tests would explicitly use test_lib.h/.c on both MSVC and GCC.

We might need to namespace the #include here to ensure it doesn't conflict with the CLSID/etc. provided by #import on MSVC. Or alternatively we need to remove the common os headers from the wrapper tests and use pure custom #includes there.

By default all the declarations are in their own namespace in the generated header. Defining INTERCOM_FLATTEN_DECLARATIONS brings all the stuff into global namespace. So by excluding that everything should be in its own namespace already.

I think we should change this so that the generated "test_lib.h" is explicitly included in the interface_wrappers.cpp instead of the compatibility header "testlib.h". I think the tests that can work with the plain IID_I* declarations should should be kept us such for now.

Fluxie commented 6 years ago

So as a result, I removed those two files from git. They are now generated through the cmake using the test/cpp-raw/CMakeLists.txt. There is also dedicated test-infra under intercom/tests/generators > that can be used for tracking the changes in the generators. I believe that was the main reason these two files were in git instead of being generated?

Another important reason was that it enabled tracking how the generated header was affected by each change in the generator. => I think we should have a copy of the generated file in the repository.

Fluxie commented 6 years ago

Was there still something missing?

I excluded the interface_wrapper tests from MSVC for now. Support for creating the objects via the Activiator class on Windows platform needs to be imlemented before they can be enabled.

Rantanen commented 6 years ago

That works. Thanks!