bytedeco / javacpp

The missing bridge between Java and native C++
Other
4.5k stars 585 forks source link

infinite recursion when calling virtualized inherited function #723

Open HGuillemet opened 1 year ago

HGuillemet commented 1 year ago
struct B {
  int x;
  virtual void f() { }
  virtual ~B() { };
};

struct D: public B {
  int y;
};
infoMap
  .put(new Info("B").virtualize())
  .put(new Info("B::x").annotations("@NoOffset"));
class Test {
  public static void main(String[] a) {
    D d = new D();
    d.f();
  }
}

This triggers an infinite recursion:

  1. Java d.f is called. f is not overridden in Java, so Java B.f is called
  2. JNI Java_B_f is called. Since B is virtualized, instead of calling C++ B::f, the object is checked whether if's an instance of the peer class JavaCPP_B. It's not the case, the object is a JavaCPP_D, and JavaCPP_D doesn't derive from JavaCPP_B. So f is called on the object, that is JavaCPP_D::f.
  3. JavaCPP_D::f calls Java d.f.

How to solve this ? Shouldn't JavaCPP_D derives from JavaCPP_B in addition of deriving from D ? Or should we call explicity B::f and bypass dynamic invocation at the end of 2. ?

saudet commented 1 year ago

It won't work if class D isn't Virtual as well, yes, that's expected.

saudet commented 1 year ago

Actually, that probably wouldn't work either. That sounds like the kind of thing your Info.upcast is supposed to solve...

HGuillemet commented 1 year ago

D is already virtual, since inherits a virtual function. The point ofupcast is to introduce static_cast or dynamic_cast when C cast doesn't work. That's not the case here. Look at my 2 suggestions at the end of the post. Doesn't one of them sound sane ?

HGuillemet commented 1 year ago

A workaround, as long as we don't need D to be virtualized, is to add, like for torch::Module:

infoMap.put(new Info("B::f").annotations("@Virtual(subclasses=false)")

This prevents the JavaCPP_D peer class to be created.