Implement cross platform API selection

Question

Implement cross platform API selection

Closed this issue 7 years ago · 5 comments

Related to #31, #18

Current plan

No features, such as winapi. Features suffer from the inheritance problem with crates. If crate B wants to use Intercom to invoke COM component, written with Intercom, without winapi and B is a dependency of A that specifies winapi feature for Intercom then B will end up using winapi as well.

Instead we will have two separate mod, with (draft) names:

intercom::platform::native
intercom::platform::generic

Effect	`...::native` + Windows	`...::native` + Linux/OSX/etc.	`...::generic` + Any OS
`extern`	`"stdcall"`	`"C"`	`"C"`
`BSTR` alloc	`SysAllocString*`	Rust heap	Rust heap
`BSTR` dealloc	`SysFreeString` or #6	#6	#6
`IErrorInfo`	`GetLastError`	???	<- Same ???
`ComRc::create()`	`CoCreateInstance`	Not available	Not affected by `mod`

This allows the Intercom user to specify per interface whether that interface should use the platform conventions or "cross-platform" conventions, which is essentially whatever we need to use on non-Windows platforms anyway as they don't have the COM infrastructure built in.

`create()` and `CoCreateInstance`

We will still have differences like ComRc::create() that will be only available on Windows. This won't be affected by the chosen intercom::compat module as ComRc is defined under the root intercom module. We could define the create() as a ComRcExt trait similar to OsStringExt, but I don't see any reason for this for now.

What makes the create()-case different from extern is that create() is purely new implementation. It doesn't change anything, like extern "C" does for extern "stdcall". As such, cross platform projects can just opt to not use it. Compiling it in on Windows doesn't prevent these projects from working on Linux if they never end up calling it.

For now we'll just #[cfg(windows)] impl the create() on ComRc and be done with it.

Answer 1 · 2017-12-22T02:59:06.000Z

Naming

Parent module name?

intercom::compat
intercom::abi
intercom::contract
intercom::platform (Obviously the mod name wouldn't be platform in this case)
intercom::convention
intercom::attrs or intercom::attributes - being explicit that it's the attributes that are affected by this.
~~intercom::extern~~ extern is a reserved word.

Platform module names?

platform/crossplatform
platform/neutral
platform/c
native/neutral
native/generic

Currently I'm leaning towards:

use intercom::platform::native::*;
use intercom::platform::generic::*;

Answer 2 · 2017-12-22T03:23:47.000Z

Implementation

And then the difficult bits: How does this work technically.

`intercom-attributes`

For each attribute, we currently have com_whatever and expand_com_whatever functions defined. The com_... is the actual #[proc_macro_attribute] function, while the expand_... is the implementation, using -> Result<..> type to allow quickly aborting with ?. So even for now we have a thin #[proc_macro_attribute] function and a fat implementing method.

We need to change this in the following way:

Introduce enum Platform { Native, Generic }
Add the platform as a parameter to expand_com_... functions.
Duplicate the #[proc_macro_attribute] fn com_... functions for Native and Generic platforms. As we can't change the function name (as that would change the attribute name) we'll do this by introducing pub mod native and pub mod generic. These functions will be otherwise identical, save for the Platform::? value they pass on to the expand_com_... function.
Carry the Platform value through to all the bits that need it. At least the following places need it:
- TyHandler/BStr needs to know how to allocate the BSTRs. This probably must be done in TyHandler as that's responsible for generating the code for the delegating methods that needs to change. We can't alter BStr as that is a static type (as opposed to attribute-expanded type) and it isn't part of the platform module.
- ReturnHandler/error needs to know how the error handling is done. We might want to keep the currently static code static, which means we'll end up passing the Platform enum over to the error module at runtime. This means the enum Platform needs to be implemented in the intercom crate as well.

In addition to the attributes, we will probably need to move IUnknown, etc. into the platform folder as well (which will be fun \:D/). This will actually make all of this more complicated.

Currently we implement IUnknown through the #[com_interface] attribute on the trait. However as we will want to keep IUnknown in the root of the intercom crate (if at all possible), we might need to remove #[com_interface] on it and instead declare the virtual tables, delegating methods, etc. by hand in the platform modules to follow the correct method invocation.

On the other hand as ComRc/etc. will want to call query_interface this will end up exploding in our face as now ComRc starts depending on the platform specific bits and needs to know what platform it is used with.

We might be able to work around this by adding trait ComInterface, which we impl on all #[com_interface] types and this trait defines which IUnknown the interface inherits from - the one using stdcall or the one using C calls.

... This just went from a simple "Just add a feature" into "Touch all parts of attribute expansion and then figure out if we can even finish this". ;_;

Answer 3 · 2017-12-22T03:48:02.000Z

IUnknown the trait needs to be platform specific.

let x = ComRc<IUnknown>;;attach( raw_com_ptr );
x.query_interface(...);  // <- ComRc needs to know how IUnknown is called.
                         // Is it extern "C" or "stdcall"

So I guess we will end up moving IUnknown to the platform specific module in intercom. This means that trait ComInterface will need to include fn platform() -> Platform member so ComRc can query the interface for its platform and thus select the correct IUnknown.

Now the question becomes whether we want platform::common, which defines IUnknown trait so that we can do the following:

mod common {
    pub trait IUnknown { ... }
}
mod native {
    #[com_interface]
    pub trait IUnknown : common::IUnknown { ... }
    impl common::IUnknown for IUnknown { /* delegates */ }
}

Which would allow us to maybe do things like impl AsRef<common::IUnknown> for ComItf<T>. This would provide IUnknown "API" in a way that isn't tied to a specific platform as the Rust code calling this doesn't care about the call conventions.

However this would only be useful in places that handle mixed platforms - which (hopefully) should be limited to Intercom internals, such as ComRc, ComItf, etc. We'll see whether this is needed when we tackle the implementation of these types with the new platform specific IUnknown traits present.

Think this is something we just need to dive into instead of trying to specify all the problems beforehand. I still feel the separate mods approach is the way to go even if a feature option would be more straight forward and simpler (as it changes the call convention in the whole crate instead of per attribute usage).

Answer 4 · 2017-12-22T03:52:23.000Z

.. on the other hand, the real question after all of this is:

Do we still want to implement the choice? The reason why this became relevant is unit tests which we would want to result in the same code on both Travis and AppVeyor.

However we could just fix the unit tests in such a way that we specify them with "stdcall" and depending on the host machine, we'll just do string replace for "stdcall" -> "C" before running the comparison if the host is one where we use "C" by default.

It's far from elegant, but it's a lot quicker and our unit tests are the only thing that would ever use intercom::platform::generic, I don't really see enough reason to complicate the code base because of it.

Answer 5 · 2017-12-22T14:25:05.000Z

Conclusion

We will not have "Cross-platform COM" and "Microsoft COM". Instead we will have "Platform COM" everywhere. This means, that unless an actual need arises, we will have:

extern "stdcall" on Windows - always.
extern "C" on other platforms - always.

Since the binaries are platform specific anyway, as long as we use a consistent call convention on each platform, the call convention shouldn't become a problem.

This leaves things like string allocation and CoCreateInstance. For this we have specific guidelines for cross-platform code. These guidelines will specify a subset of Intercom functionality that we will guarantee to work on every platform:

BSTRs are allocated using #6 and they must be de-allocated using #6.
Non-BSTR pointer values through IMalloc.
- In practice this means the COM clients will need to CreateInstance CLSID_Allocator or similar type that they can then use to alloc/dealloc any pointers they receive from the same Intercom typelib.
Do not use CoCreateInstance (or ::create() in Rust)

As long as people follow these guidelines, they will be able to write components using Intercom (Intercomponents!) that work cross-platform without code changes.

Note that these guidelines should not prevent us from making these exact same components work well in platform specific environments. For example BSTRs allocated using #6 does not mean that #6 can't allocate them using SysAllocString on Windows, which allows the same components to be used with Windows-specific COM clients, such as C# projects.

In addition we can provide CoCreateInstance based ::create( clsid ) method in Intercom, but using this means the code will work only on platforms that support CoCreateInstance.

So in short: Instead of having "native COM" and "generic COM" as two differing implementations, our "generic COM" will be a subset of "native COM". In addition we will give up "generic COM" being the same on all platforms. It will have the same API on all platforms, but may have different ABI thanks to compiler differences.

I believe for now we have a rather good understanding of what cross-platform Intercom means, so I'm also closing #23.