haxscramper / hcparse

High-level nim bindings for parsing C/C++ code
https://haxscramper.github.io/hcparse-doc/src/hcparse/libclang.html
Apache License 2.0
37 stars 2 forks source link

Reusing nim `method` for wrapping C++ methods #1

Open haxscramper opened 3 years ago

haxscramper commented 3 years ago

When interfacing with C++ classes and methods, it is necessary to re-wrap methods as procedures for each derived class. E.g. if I have Base and Derived I would have to repeat wrapping of the Base methods for Derived type. I tried using method, but it gives me 'invalid pragma: importcpp: "#.toOverride(@)"' error on the following code, so it looks like I can't benefit from nim OOP features in this case.

  {.emit: """/*TYPESECTION*/
struct Base {
  int baseField;
  int baseMethod() { return 12; }
  virtual int toOverride() { return 42; }

};
struct Derived : public Base {
  int derivedField;
  int toOverride() override { return 45; }
};
""".}

type
  CxxBase {.importcpp: "Base", bycopy, inheritable.} = object
    baseField: cint

  CxxDerived {.importcpp: "Derived", bycopy.} = object of CxxBase
    derivedField: cint

# All methods of the base class must be repeated for each derived. For
# cases like `QWidget` that has 214 base methods and 37 derived it classes results
# in **7918!** repetitions.
proc baseMethod(base: CxxBase): cint {.importcpp: "#.baseMethod(@)".}
proc baseMethod(base: CxxDerived): cint {.importcpp: "#.baseMethod(@)".}

proc toOverride(base: CxxBase): cint {.importcpp: "#.toOverride(@)".}
proc toOverride(base: CxxDerived): cint {.importcpp: "#.toOverride(@)".}

proc toOverrideTypeclass(base: CxxBase | CxxDerived): cint {.importcpp: "#.toOverride(@)".}

# Ideally I would like to import C++ methods as **methods**, but I get
# 'Error: invalid pragma: importcpp: "#.toOverride(@)"'

when false:
  method toOverrideMethod(base: CxxBase): cint {.importcpp: "#.toOverride(@)".}

block:
  let derived = CxxDerived(baseField: 123123)
  echo derived.baseField

  echo CxxBase().toOverride()
  echo CxxDerived().toOverride()

  echo CxxBase().toOverrideTypeclass()
  echo CxxDerived().toOverrideTypeclass()

type
  NimBase {.inheritable.} = object
    baseField: cint

  NimDerived = object of NimBase
    derivedField: cint

method baseMethod(base: NimBase): int {.base.} = 12
method toOverride(base: NimBase): int {.base.} = 42
method toOverride(base: NimDerived): int = 45

block:
  let derived = NimDerived(baseField: 123123)
  echo derived.baseField

  echo NimBase().toOverride()
  echo NimDerived().toOverride()

I specifically repeated the implementation in nim as well to illustrate how I would like wrapper to behave. It is not difficult to generate additional wrappers for all derived classes, but as indicated in comment it could lead to multi-thousand repetitions (again QWidget has 37 derived classes, and it would repeat at least 214 methods for each of them. And I used QWidget as and example. With QObject it could be even bigger).

I thought about possible solutions, but they don't seem particularly plausible to me:

  1. Using typeclasses - proc toOverride(base: CxxBase | CxxDerived): cint {.importcpp: "#.toOverride(@)".}. It might look fine for such a small example, but in the end it would mean putting all derived class wrappers in a single file, and declare all procedures afterwards, creating one giant file from a library instead of keeping things separated.
  2. Implementing wrappers as a macro that checks if the argument has been derived from required base class - this might work, but it seems like a horrible hack where I have to mix {.emit.} with macro code and traverse full inheritance tree on each call encountered in code.

So the question is - can method somehow be used to solve this issue, or I need to generate repeated wrappers for each derived class (right now I'm doing this, which results in tons of repetitions)?


Originally asked in forum thread, but also copied here because it is particularly important, but unlikely to be answered soon.

timotheecour commented 3 years ago

the following "almost works":

when true:
  {.emit: """/*TYPESECTION*/
  struct Base {
    int baseField;
    int baseMethod() { return 12; }
    virtual int toOverride() { return 42; }

  };
  struct Derived : public Base {
    int derivedField;
    int toOverride() override { return 45; }
  };
  """.}

  type
    CxxBase {.importcpp: "Base", bycopy, inheritable.} = object
      baseField: cint

    CxxDerived {.importcpp: "Derived", bycopy.} = object of CxxBase
      derivedField: cint

  proc baseMethodImpl(base: CxxBase): cint {.importcpp: "#.baseMethod(@)".}
  method baseMethod(base: CxxBase): cint {.base.} = baseMethodImpl(base)
  proc toOverrideImpl(base: CxxBase): cint {.importcpp: "#.toOverride(@)".}
  method toOverride(base: CxxBase): cint {.base.} = toOverrideImpl(base)
  when defined case2b:
    proc toOverrideImpl2(base: CxxDerived): cint {.importcpp: "#.toOverride(@)".}
    method toOverride(base: CxxDerived): cint = toOverrideImpl2(base)

  block:
    let derived = CxxDerived(baseField: 123123)
    echo derived.baseField

    echo CxxBase().baseMethod()
    echo CxxDerived().baseMethod()

    echo CxxBase().toOverride()
    when defined case2b:
      echo CxxDerived().toOverride()

nim r -b:cpp main works nim r -b:cpp -d:case2b main gives cgen error:

/Users/timothee/git_clone/nim/timn/build/nimcache/@mt12445.nim.cpp:222:14: error: no member named 'm_type' in 'Base'
                if (!(base.m_type == (&NTIcxxderived__5KCvJiTlz3Lz1sFqYEsBSQ_))) goto LA3_;
                      ~~~~ ^
/Users/timothee/git_clone/nim/timn/build/nimcache/@mt12445.nim.cpp:223:20: error: no member named 'm_type' in 'Base'
{               if (!isObj(base.m_type, (&NTIcxxderived__5KCvJiTlz3Lz1sFqYEsBSQ_))){ raiseObjectConversionError(); }
                           ~~~~ ^
/Users/timothee/git_clone/nim/timn/build/nimcache/@mt12445.nim.cpp:230:29: error: no member named 'm_type' in 'Base'
                if (!(isObjWithCache(base.m_type, (&NTIcxxbase__AnOdMbGZliGJzFUoc6fCJA_), Nim_OfCheck_CACHE3))) goto LA6_;
                                     ~~~~ ^
haxscramper commented 3 years ago

This issue can be partially solved if only ref/ptr types are used. It still breaks on the value objects as things just break on C++ semantics (when passing bycopy objects to a procedure)

{.emit: """/*TYPESECTION*/
struct Base {
  int baseField;
  int baseMethod() { return 12; }
  virtual int toOverride() { return 42; }

};
struct Derived : public Base {
  int derivedField;
  int toOverride() override { return derivedField; }
};

struct Other {};
""".}

type
  CxxBase {.importcpp: "Base", bycopy, inheritable.} = object
    baseField: cint

  CxxDerived {.importcpp: "Derived", bycopy.} = object of CxxBase
    derivedField: cint

  CxxOther {.importcpp: "Other", bycopy.} = object

proc aux() {.header: "<new>", importc: "//".}

proc toOverride(base: ref CxxBase): cint {.importcpp: "#->toOverride(@)".}
  # {.emit: "return `base`->toOverride();".}

proc newCxxBase(): ref CxxBase  =
  aux()
  new(result)
  {.emit: "new ((void*)result) Base();".}

proc newCxxDerived(): ref CxxDerived  =
  aux()
  new(result)
  {.emit: "new ((void*)result) Derived();".}

proc newCxxOther(): ref CxxOther =
  aux()
  new(result)
  {.emit: "new ((void*)result) Other();".}

echo newCxxBase().toOverride()
echo newCxxDerived().toOverride()
# echo newCxxOther().toOverride()
haxscramper commented 3 years ago

Actually I just realized I can drop bycopy and replace it with byref:

{.emit: """/*TYPESECTION*/
struct Base {
  int baseField;
  int baseMethod() { return 12; }
  virtual int toOverride() { return 42; }

};
struct Derived : public Base {
  int derivedField;
  int toOverride() override { return derivedField; }
};

struct Other {};
""".}

type
  CxxBase {.importcpp: "Base", byref, inheritable.} = object
    baseField: cint

  CxxDerived {.importcpp: "Derived", byref.} = object of CxxBase
    derivedField: cint

  CxxOther {.importcpp: "Other", byref.} = object

proc aux() {.header: "<new>", importc: "//".}

method toOverride(base: CxxBase): cint {.base.} =
  {.emit: "return `base`->toOverride();".}

echo CxxBase().toOverride()
echo CxxDerived().toOverride()
# echo newCxxOther().toOverride()

now works corectly

timotheecour commented 3 years ago

cool! nim manual is in desperate need for a beefed up section (ideally a separate rst file, eg interop.rst) dedicated to interop, are you interested in a PR that would show this example?

needs a bit of cleanup, eg remove aux and add

  var a = CxxDerived()
  a.derivedField = 123
  assert a.toOverride() == 123
haxscramper commented 3 years ago

Feel free to add it to any manual section, but after releasing alpha version of hcparse I wanted to spend some time summarizing all my solution in a series of articles, something like "Nim for C++ programmer". Also, I'm not really sure I'd this is the correct approach, so I'd prefer to mass-test it on some library (like Qt, which was the reason for this question) and then say "yes, this is the correct way that would not bite you on some edge case"

timotheecour commented 3 years ago

but in the meantime it's good to show what's possible to unblock ppl that need interop; it's more of a living document given that C++ interop is still in flux

haxscramper commented 3 years ago
method toOverride(derived: CxxDerived2): cint =
  echo "Overide method impementation from nim side"
  return 1231.cint

echo newCxxDerived2().toOverride()

does not work as it fails with C++ codegen compilation error

/mnt/workspace/clean-clone/hack/testing/nim/c_interop/cpp_tests/cache/@minheritance.nim.cpp:330:31: error: ‘struct Base’ has no member named ‘m_type’
  330 |                 if (!((*base).m_type == (&NTI__5WdQBNAaXBHGF2AaoSDNJQ_))) goto LA3_;
timotheecour commented 3 years ago

i see, so it's same error as above (https://github.com/haxscramper/hcparse/issues/1#issuecomment-865166616)

maybe file an issue in nim repo? RFC or not RFC, this should be fixed (and bugs with a tracking issue are more likely to be addressed, at very least gives a centralized placeholder for discussion)

haxscramper commented 3 years ago

Well, the issue seems obvious - object is not derived from a RootObj so it does not have an m_type field. Since I also want to derive from C++ issue it would be unsolvable without multiple inheritance.

So I will refrain from filing issues on this since it would be incoherent noise IMO, this need to be addressed in an RFC

timotheecour commented 3 years ago

it would be unsolvable without multiple inheritance.

there should be a solution for this case that doesn't require solving MI (which is more complex and can be addressed seperately), ie allowing (possibly with a pragma) method toOverride(base: CxxDerived2): cint = 33 to work even though CxxDerived2 doesn't derive from a RootObj; ie deriving from an importcpp inheritable object should be enough

(and yes, an RFC would be good)

haxscramper commented 2 years ago

Alternatively to hacks (generating byref types or bruteforce solutions with copypasting every method) I can just require explicit type conversion using 'as' operator -

var slider = cnewQSlidwe()
slider.as(QWidget)[].someQWidetMethod()