autowrap icon indicating copy to clipboard operation
autowrap copied to clipboard

Cannot generate python wrapper for eventcore.core.EventDriver using PyNIH

Open imzhaodong opened this issue 3 years ago • 18 comments

Autowrap cannot generate python wrapper for interface eventcore.core.EventDriver correctly using PyNIH. The client code will be like below to be wrapped:

import eventcore.core : EventDriver;
EventDriver hack3() { return EventDriver.init; }

And after compile, when load it in Python, an undefined symbol error message will be printed out:

ImportError: /home/dzhao/development/autohello/build/pythonlib/adder.so: undefined symbol: _D6python4type__T11PythonClassTC9eventcore6driver10RefAddressZQBt__T11propertyGetS_DQCaQBtQBp7nameLenMxFNaNbNdNiNfZkZQBxUNbPSQEt3raw7_objectPvZQu

imzhaodong avatar Jul 13 '20 02:07 imzhaodong

To be clear, the issue is not happened at compile or link time. The issue happened when you import the result binary library into Python, and then you see the issue undefined symbol error.

imzhaodong avatar Jul 15 '20 10:07 imzhaodong

I dig in further and I think I know what had happened. For any module or class or struct or mixin template, there need to be one function there. If there is no function to be wrapped, it will cause error. Not compile or link error, but error when Python load this lib. Code sample 1 to repro issue:

module x;
class Problem {
} //this will cause error. The way to fix it is add a function into this module

Code Sample 2 to repro issue:

void hack41() {}
struct S2 {
    //this will have issue, because no method in this class
}
struct Problem {
    void add(S2 d) {}
}

imzhaodong avatar Jul 16 '20 05:07 imzhaodong

After the fixing the original issue solved and now it cannot wrap below code.

  import eventcore.driver : EventDriverSignals;
  EventDriverSignals hack31() {return EventDriverSignals.init;}

Same compile and link is OK, but when load it into Python then this symbol cannot be found:

python.raw._object* python.type.callDlangFunction!(python.type.callDlangFunction!(eventcore.driver.SignalListenID, eventcore.driver.SignalListenID.__mixin1.__ctor(ulong, uint)).callDlangFunction(python.raw._object*, python.raw._object*, python.raw._object*).__lambda5(ulong, uint), std.typecons.Tuple!(ulong, uint).Tuple).callDlangFunction(ref std.typecons.Tuple!(ulong, uint).Tuple)

imzhaodong avatar Jul 17 '20 10:07 imzhaodong

Probably from vibed?

Laeeth avatar Jul 17 '20 22:07 Laeeth

So in summary we have more than one issue here: The first issue is when there is zero function in a module/class/struct/mixin, the autowrap is not able to wrap it. For this one, Atila has provided a workaround which is disable check at file in autowrap project pynih/source/python/cooked.d line 51 change static if(__traits(compiles, PythonType!T.pyType)) { to static if(true) {.

I have figured out the 2nd issue which is why we cannot wrap 3rd party library class eventcore.driver.EventDriverSignals. This is because in autowrap project file pynih/source/python/conv/python_to_d.d line 300, when wrap a function marked as @safe, this line will try to convert a @system function to a @safe fuction which is not valid. A better fix should be modify the implementation to make it @safe. For me to work it around quickly, I now just changed this line to return (UnqualParams dArgs) @trusted { which I have added @trusted here. The way to repro this issue is to try to wrap a file like below and you will see what I'm saying and why my proposed work around is working.

import eventcore.driver : Handle, SignalStatus;
alias SignalCallback2 = void delegate(SignalListenID2, SignalStatus, int) @safe;
struct SignalListenID2 { mixin Handle!("signal", size_t, size_t.max); }
class EventDriverSignals2 {
    void listen(SignalCallback2 on_signal) {}
    void addRef(SignalListenID2 descriptor) {}
}

It looks there are still other issues to wrap class vibe.core.net.TCPConnection even even after we can work around above two issues. I will post it here later after I try to narrow it down as much as I can.

imzhaodong avatar Jul 20 '20 04:07 imzhaodong

The third issue looks is we cannot wrap type type epoll_data_t successful. We will run into compiler error like below:

../autowrap/pynih/source/python/type.d(889,24): Error: no property fieldNames for type python.type.PythonType!(epoll_data_t)
../autowrap/pynih/source/python/type.d(890,24): Error: no property fieldTypes for type python.type.PythonType!(epoll_data_t)
../autowrap/pynih/source/python/type.d(85,36): Error: template instance python.type.PythonClass!(epoll_data_t) error instantiating
../autowrap/pynih/source/python/type.d(842,18):        instantiated from here: PythonType!(epoll_data_t)                                     ../autowrap/pynih/source/python/conv/d_to_python.d(74,23):        instantiated from here: pythonClass!(epoll_data_t)
../autowrap/pynih/source/python/type.d-mixin-848(848,21):        instantiated from here: toPython!(epoll_data_t)
../autowrap/pynih/source/python/conv/d_to_python.d(74,23):        ... (4 instantiations, -v to show) ...                                     

And if we look at the definition of epoll_data_t, here is its definition according to

typedef union epoll_data
{
  void *ptr;
  int fd;
  uint32_t u32;
  uint64_t u64;
} epoll_data_t;

When I check the various implementation of function PyObject* toPython()() in file pynih/source/python/conv/d_to_python.d line 101, it mentioned that convert dlang void* to Python is not supported yet. This looks the ultimate issue.

imzhaodong avatar Jul 20 '20 04:07 imzhaodong

Atila has sent me a patch for DMD to see if it can work around. I paste it here to keep a record.

diff --git a/src/dmd/dtemplate.d b/src/dmd/dtemplate.d
index 619cc0611..7159e3855 100644
--- a/src/dmd/dtemplate.d
+++ b/src/dmd/dtemplate.d
@@ -6186,6 +6186,41 @@ extern (C++) class TemplateInstance : ScopeDsymbol
      */
     final bool needsCodegen()
     {
+        if(minst) return true;
+
+        if (!minst)
+        {
+            // If this is a speculative instantiation,
+            // 1. do codegen if ancestors really needs codegen.
+            // 2. become non-speculative if siblings are not speculative
+
+            TemplateInstance tnext = this.tnext;
+            TemplateInstance tinst = this.tinst;
+            // At first, disconnect chain first to prevent infinite recursion.
+            this.tnext = null;
+            this.tinst = null;
+
+            // Determine necessity of tinst before tnext.
+            if (tinst && tinst.needsCodegen())
+            {
+                minst = tinst.minst; // cache result
+                assert(minst);
+                //assert(minst.isRoot() || minst.rootImports());
+                return true;
+            }
+            if (tnext && (tnext.needsCodegen() || tnext.minst))
+            {
+                minst = tnext.minst; // cache result
+                assert(minst);
+                //return minst.isRoot() || minst.rootImports();
+                return true;
+            }
+
+            // Elide codegen because this is really speculative.
+            return false;
+        }
+
+
         // Now -allInst is just for the backward compatibility.
         if (global.params.allInst)
         {
@@ -6281,7 +6316,7 @@ extern (C++) class TemplateInstance : ScopeDsymbol
             return false;
         }
 
-        if (global.params.useUnitTests)
+        if (global.params.useUnitTests && false)
         {
             // Prefer instantiations from root modules, to maximize link-ability.
             if (minst.isRoot())

imzhaodong avatar Jul 23 '20 04:07 imzhaodong

Unfortunately this custom compiler will crash when compile PyNIH for vibe.core.net.TCPConnection class. The code which can repro this being checked on branch bug5_pynih_tcp_not_work https://git.kaleidic.io/SIL/plugins/stable/carbon-d/-/tree/bug5_pynih_tcp_not_work

imzhaodong avatar Jul 23 '20 04:07 imzhaodong

I think something is wrong that you are ending up wrapping eventcore at all

Laeeth avatar Jul 23 '20 06:07 Laeeth

Start with just wrapping a simple module with just structs. Make it work in python.

Then wrap carbon client. It might be it tries recursively to wrap vibed types in which case remove mongo and redis vibe versions and I don't think vibe will be used at all. Getting auto wrsp to work for vibed is much harder problem

Laeeth avatar Jul 23 '20 06:07 Laeeth

For everyone's info. Atila has released a new version of Autowrap package into Dub. Together with that and the custom DMD compiler, now we are at least able to wrap some part of the code successful (which we cannot do in the past), this code is at branch autowrap-from-scratch. https://git.kaleidic.io/SIL/plugins/stable/carbon-d/-/tree/autowrap-from-scratch

Though the part we really care which is CarbonClient, we are not there yet.

imzhaodong avatar Jul 27 '20 03:07 imzhaodong

Also I have a different branch tinyredis for Carbon-D project, which I have already migrated to use tinyredis and mongd. And when I compile it use the latest autowrap version and my custom DMD, I will run into an assertion error and then DMD core dumped. Below is part of the error message looks like.

WARNING: could not wrap aggregate symmetry.api.rabbitmq.bindings.amqp_field_value_t

ERROR: This is a compiler bug.
Please report it via https://issues.dlang.org/enter_bug.cgi
with, preferably, a reduced, reproducible example and the information below.
DustMite (https://github.com/CyberShadow/DustMite/wiki) can help with the reduction.
---
DMD v2.093.0-218-g3ed1f37a9-dirty
predefs   StdLoggerDisableTrace EmitCSharp Python PythonExtension PyNIH Have_symmetry_carbon Have_asdf Have_autowrap_pynih Have_core_json Have_
core_msgpack Have_dateparser Have_dproto Have_mondo Have_msgpack_d Have_rabbitmq Have_requests Have_sil_lang Have_sspi_d Have_thrift_d_kaleidic
 Have_tinyredis Have_autowrap_common Have_autowrap_reflection Have_mirror Have_pegged Have_sil_parser Have_emsi_containers Have_stdx_allocator
UseVibe Have_painlesstraits Have_vibe_d Have_vibe_d_core Have_vibe_core Have_vibe_d_data Have_vibe_d_utils Have_vibe_d_crypto Have_vibe_d_data
Have_vibe_d_http Have_vibe_d_inet Have_vibe_d_mail Have_vibe_d_mongodb Have_vibe_d_redis Have_vibe_d_stream Have_vibe_d_textfilter Have_vibe_d_
tls Have_vibe_d_utils Have_vibe_d_web Have_eventcore EventcoreEpollDriver Have_taggedalgebraic Have_mir_linux_kernel Have_diet_ng Have_openssl
Have_cachetools Have_libevent DigitalMars Posix linux ELFv1 CRuntime_Glibc CppRuntime_Gcc LittleEndian D_Version2 all D_SIMD D_InlineAsm_X86_64
 X86_64 D_LP64 D_PIC assert D_ModuleInfo D_Exceptions D_TypeInfo D_HardFloat
binary    /home/dzhao/dlang/mydmd/dmd/generated/linux/release/64/dmd
version   v2.093.0-218-g3ed1f37a9-dirty
config    /home/dzhao/dlang/mydmd/dmd/generated/linux/release/64/dmd.conf
DFLAGS    -I/home/dzhao/dlang/mydmd/dmd/generated/linux/release/64/../../../../../druntime/import -I/home/dzhao/dlang/mydmd/dmd/generated/linux
/release/64/../../../../../phobos -L-L/home/dzhao/dlang/mydmd/dmd/generated/linux/release/64/../../../../../phobos/generated/linux/release/64 -
L--export-dynamic -fPIC
---
core.exception.AssertError@dmd/hdrgen.d(2292): Assertion failure
----------------
??:? _d_assertp [0x555a5a7ffd81]
dmd/hdrgen.d:2292 _ZN28ExpressionPrettyPrintVisitor5visitEP8CommaExp [0x555a5a62eb98]

imzhaodong avatar Jul 27 '20 04:07 imzhaodong

Here is the latest status:

  1. Atila has created a new file source/symmetry/carbon/autowrap.d to do wrap from scratch and using a minimal pollution philosophy, this code is already in master branch.
  2. I have switch to use TinyRedis lib from Vibe-d:Redis lib, this can solve the issue of wrap CarbonRedisClient class.
  3. Atila has released new version of mirror library and autowrap library into Dub repository, this can solve the issue of wrap RabbitClient class.
  4. With above progress. more work is still needed to wrap CarbonClient. At this moment if we wrap CarbonClient the DMD will core dump. This code is under branch wrap_carbon_client.

imzhaodong avatar Jul 28 '20 04:07 imzhaodong

I also spend more time on narrow down why compile CarbonClient will cause issue. There are at least two issues. One is it will cause DMD core dump which I do not know yet. For another I have narrowed it down, any function return kaleidic.sil.lang.types.Variable will cause compile/link OK but load in Python time symbol error. I have checked this code into branch bug9_can_not_wrap_sil_variable. Basically if you put below code into source/symmetry/carbon/autowrap.d it will cause Python load time symbol problem:

alias Variable = from!"kaleidic.sil.lang.types".Variable;
export Variable hack51()
{
    return Variable.init;
}

imzhaodong avatar Jul 28 '20 06:07 imzhaodong

Atila has pointed out that when we wrap CarbonClient, if we use dmd 2.092.0 then it will be core dump, but if we use 2.091.1 then it will be not. This is very helpful because then at least for now we do not need to depend on custom DMD compiler. Stefan pointed out that this is highly likely because 2.091.1 version DMD still implicitly use -allinst option for DMD. So now if use this version 2.091.1 DMD on branch wrap_carbon_client and wrap CarbonClient, we will be able to compile/link but run into below error when load it into Python:

ImportError: /home/dzhao/development/autohello/build/pythonlib/carbon2.so: undefined symbol: _D6python4type__T20PythonBinaryOperatorTS3std8typecons__T8NullableTSQBb8datetime4date4DateZQBiVS6mirror4meta6traits14BinaryOperatorS2a1_2bi1Z12_py_bin_funcUNbPSQGc3raw7_objectQrZQu

And this is to complain below symbol:

extern (C) nothrow python.raw._object* python.type.PythonBinaryOperator!(std.typecons.Nullable!(std.datetime.date.Date).Nullable, mirror.meta.traits.BinaryOperator("+", 1))._py_bin_func(python.raw._object*, python.raw._object*)

imzhaodong avatar Jul 28 '20 13:07 imzhaodong

I found that autowrap cannot wrap a quite simple struct BondEntry successful. I think this should help to identify why autowrap cannot wrap it. My code is at branch bug10_can_not_wrap_bondentry Basically here is my code to be wrapped.

alias BondEntry = from!"symmetry.carbon.clienthelper".BondEntry;
And here is the definition of BondEntry:
struct BondEntry
{
    import std.typecons : Nullable;

    @Name("floaterFlag")
    bool isFloater;

    @Name("maturityDt")
    Date maturityDate;

    @Name("issueDt")
    Date issueDate;

    @Name("effectiveDt")
    Date effectiveDate; // CHECKME

    string isin;

    @Name("ccy")
    string currency;

    @Name("cpnRate")
    double couponRate;

    @Name("firstCpnDt")
    Nullable!Date firstCouponDate;

    @Name("auctionDt")
    Nullable!Date auctionDate;

    @Name("mkt")
    string market;

    @Name("floaterFlag")
    bool isLinker;

    @Name("ric")
    string ric;
}

And I got into this symbol error:

extern (C) nothrow python.raw._object* python.type.PythonBinaryOperator!(std.typecons.Nullable!(std.datetime.date.Date).Nullable, mirror.meta.traits.BinaryOperator("+", 1))._py_bin_func(python.raw._object*, python.raw._object*)

imzhaodong avatar Jul 29 '20 13:07 imzhaodong

I have just approved one thing. For wrapping CarbonClient class, if I mark all method which return kaleidic.sil.lang.types.Variable or BondEntry methods as package rather than public, I'm able to compile the code and load it into Python without symbol error.

imzhaodong avatar Jul 30 '20 02:07 imzhaodong

And for issue of BondEntry, I narrow it down further to Nullable!Date cannot be wrapped. Nullable and Date are all from Phobos library so this looks a autowrap bug. I have checked in my code at branch bug11_can_not_wrap_nullable_date. Basically below simple code cannot be wrapped by autowrap, cause loading symbol error when load it into Python.

export struct Hack61
{
    import std.typecons : Nullable;
    import std.datetime : Date;
    Date m1; //OK
    Nullable!int m2; //OK
    Nullable!Date m3; //not OK
}

imzhaodong avatar Jul 30 '20 04:07 imzhaodong