* As per my offline comment and as Jake had pointed out, you can cast pointers to incomplete types, this will leverage the C type system better than mere typedefs, and enforce a modicum of an interface’s contract. * C does not have namespaces or overloading, so we will need a naming convention to differentiate our types and methods from any other library or the application code. That means all types and functions should be prepended with “geode_” or something similar. * We must absolutely produce a dynamic library. Not all FFI’s support static linking. This will also give us greater control over the symbols being exported. I agree that this dynamic link target should not share it’s symbol space with the C++ library. This grants much greater flexibility * I recommend we avoid introducing real types in the exported interface. In order to support future revision, we’d have to introduce version and size information into the struct a la THE WIN32 API. Remember that? Having to zero out the struct then setting the size and version members before the other members? This is why they did that. * The implementation needs to guard against throwing exceptions across the library boundary. * This library needs a thread safe means of error handling. There is not enough fleshed out in this RFC to have a meaningful conversation about this at this time. * I would like to see an ability for the client to specify their own allocators. This would require an overhaul of the existing C++ ABI and may have an impact on our dependencies. This is another talk for a more complete RFC.
> On Mar 27, 2020, at 8:52 AM, Jacob Barrett <jbarr...@pivotal.io> wrote: > > Another couple of thing I thought of last night. > > The RFC should probably include mentions of tools like SWIG or Clang > libtooling for auto generating C-bindings for C++ headers. > > One important area that isn’t covered is how this would address serialization > of types/objects in the bound language. For example, if I used this to bind > to Go how would my Go object be passed to put, get, or any other method that > takes a serializable object? When received how is this language going to be > invoked to de-serialize? More time needs to be spent on this aspect of the > interface. > > -Jake > > >> On Mar 26, 2020, at 3:10 PM, Jacob Barrett <jbarr...@pivotal.io> wrote: >> >> Forgot to include the source example: >> https://github.com/pivotal-jbarrett/geode-native/tree/e47698bad3cf725dcbddaad458813e26e75c8c71/ccache >> >> <https://github.com/pivotal-jbarrett/geode-native/tree/e47698bad3cf725dcbddaad458813e26e75c8c71/ccache> >> >> >>> On Mar 26, 2020, at 2:56 PM, Jacob Barrett <jbarr...@pivotal.io >>> <mailto:jbarr...@pivotal.io>> wrote: >>> >>> On the strongly typed, I found a solution I like even better. >>> >>> struct CacheFactory; >>> typedef struct CacheFactory CacheFactory; >>> >>> struct Cache; >>> typedef struct Cache Cache; >>> >>> CacheFactory* createCacheFactory() { >>> return reinterpret_cast<CacheFactory*>( >>> new apache::geode::client::CacheFactory()); >>> } >>> >>> void destroyCacheFactory(CacheFactory* cacheFactory) { >>> delete >>> reinterpret_cast<apache::geode::client::CacheFactory*>(cacheFactory); >>> } >>> >>> Cache* createCache(CacheFactory* cacheFactory) { >>> return reinterpret_cast<Cache*>(new apache::geode::client::Cache( >>> reinterpret_cast<apache::geode::client::CacheFactory*>(cacheFactory) >>> ->create())); >>> } >>> >>> void destroyCache(Cache* cache) { >>> delete reinterpret_cast<apache::geode::client::Cache*>(cache); >>> } >>> >>> The code below is type safe and won’t compile. >>> CacheFactory cacheFactory = createCacheFactory(); >>> destroyCache(cacheFactory); >>> >>> >>> -Jake >>> >>> >>>> On Mar 26, 2020, at 12:22 PM, Jacob Barrett <jbarr...@pivotal.io >>>> <mailto:jbarr...@pivotal.io>> wrote: >>>> >>>> I am onboard with the idea but I would like more details in the RFC. >>>> >>>> I would prefer that the C bindings be in its own library. The only symbols >>>> that should be visible from this library should be the C exported symbols, >>>> the internal C++ symbols should be hidden. >>>> >>>> I feel like this library makes the most sense as a static library. I saw >>>> this because in .NET we would link it as a mixed mode assembly to avoid >>>> having multiple shared libraries to link. If we did a node client I would >>>> expect it too would statically link this library as into its shared >>>> library. Same I imagine would apply to all other language bindings. >>>> >>>> How are you planning on managing allocation and deallocation of these >>>> opaque void* pointers. You vaguely showed the allocation but deallocation >>>> would be a good example. I would assume some “destroy” method for every >>>> “create” method. >>>> >>>> Does it make sense to have the CacheFactory concept at all (especially >>>> since it is more of a builder)? Could we have some C struct that can be >>>> used to create the cache, where the struct has fields for all the >>>> configuration? In general can we rethink the API so that it makes sense >>>> for C or other language bindings? >>>> >>>> What about finding a way to be more expressive than void*. Typedefs at >>>> least express some meaning when hovering but will just decay without >>>> warning to the void*. >>>> >>>> Consider: >>>> typedef void* CacheFactory; >>>> typedef void* Cache; >>>> CacheFactory createCacheFactory(void); >>>> void destroyCacheFactory(CacheFactory cacheFactory); >>>> Cache createCache(CacheFactory cacheFactory); >>>> void destroyCache(Cache cache); >>>> >>>> But this compiles: >>>> CacheFactory cacheFactory = createCacheFactory(); >>>> destroyCache(cacheFactory); >>>> >>>> And explodes at runtime. >>>> Example >>>> https://github.com/pivotal-jbarrett/geode-native/tree/2d7d0a28fb6f85988e7b921fd96ebb975bad8126/ccache >>>> >>>> <https://github.com/pivotal-jbarrett/geode-native/tree/2d7d0a28fb6f85988e7b921fd96ebb975bad8126/ccache> >>>> >>>> We could use structs to hold the opaque pointers, and other values. >>>> typedef struct { >>>> void* ptr; >>>> } CacheFactory; >>>> typedef struct { >>>> void* ptr; >>>> } Cache; >>>> >>>> And now this does not compile: >>>> CacheFactory cacheFactory = createCacheFactory(); >>>> destroyCache(cacheFactory); >>>> Example >>>> https://github.com/pivotal-jbarrett/geode-native/tree/23b44e281fd9771474ff3555020710bf23f454ae/ccache >>>> >>>> <https://github.com/pivotal-jbarrett/geode-native/tree/23b44e281fd9771474ff3555020710bf23f454ae/ccache> >>>> >>>> >>>> -Jake >>>> >>>> >>>>> On Mar 24, 2020, at 2:20 PM, Blake Bender <bben...@pivotal.io >>>>> <mailto:bben...@pivotal.io>> wrote: >>>>> >>>>> Hello everyone, >>>>> >>>>> We'd like to add C bindings to the native client, in preparation for the >>>>> larger task of adding support for new languages, e.g. .net core. Please >>>>> have a look at the proposal below and let me know what you think. >>>>> >>>>> Thanks, >>>>> >>>>> Blake >>>>> >>>>> >>>>> https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library >>>>> >>>>> <https://cwiki.apache.org/confluence/display/GEODE/Add+C+Bindings+to+Native+Client+Library> >>>> >>> >> >
