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>
>>> 
>> 
> 

Reply via email to