Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

infinite recursion when calling virtualized inherited function #723

Open
HGuillemet opened this issue Oct 21, 2023 · 4 comments
Open

infinite recursion when calling virtualized inherited function #723

HGuillemet opened this issue Oct 21, 2023 · 4 comments

Comments

@HGuillemet
Copy link
Contributor

HGuillemet commented Oct 21, 2023

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
Copy link
Member

saudet commented Oct 21, 2023

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

@saudet
Copy link
Member

saudet commented Oct 21, 2023

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

@HGuillemet
Copy link
Contributor Author

HGuillemet commented Oct 21, 2023

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
Copy link
Contributor Author

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants