binder icon indicating copy to clipboard operation
binder copied to clipboard

Inheritance of inner classes

Open ThFe opened this issue 4 years ago • 2 comments

Hi @lyskov,

Trying bind the code

struct outer {
	struct inner {
		int a;
	};
};

struct outer2 : public outer::inner {
	double b;
};

currently fails, giving the error "ERROR: Could not find binder for type: ". The reason is that - while outer::inner is registered as a dependency of outer2 - there is no Binder object for outer::inner. (Bindings for outer::inner are generated when processing struct outer, though.)

This pull request solves this problem by:

  1. introducing an simple Binder class called Dependency which for a given type (like outer::inner) depends on the type (like outer) whose Binder object actually provides the bindings and
  2. registers for inner classes appropriate Dependency objects with the context object.

Regards, Thomas

ThFe avatar May 24 '21 18:05 ThFe

Hi @lyskov,

DiscountingSwapEngine gives an example of such a design (inheriting from an inner class). Whether this is an example of a good design is another question...

In general, I do share your concerns regarding such a design. On the other hand, C++ supports it and binding code for inner classes is already generated by binder (e.g., T15.inner_class.hpp). I therefore thought it good to have binder supporting this as well (I am currently facing this problem in a project where I do not have influence on the design).

In the meantime, I realised that the proposed solution may lead to Python import errors when two or more classes have identically named inner classes (the Python name would be the same then, leading to import errors).

In order to resolve this problem, I would propose to change binder such as to

  1. not binding inner classes by default (needs to be required via +class config settings) and
  2. allowing custom python class names (e.g., by adding an optional argument to +class ClassName [PythonName]).

First point would change binder's behaviour, second point could be useful as well for templated classes.

[I could provide a proposal for the required code changes.]

ThFe avatar Jun 16 '21 19:06 ThFe

@ThFe thank you for info! I agree that it would be preferable to be able generate bindings for everything that C++ technically supports.

re 1, 2: this might work for small/medium projects but for large code bases it might not be practical to require explicit listing of classes. Hm... it looks like the core issue here is that right now inner classes is bound when umbrella class is bound. So making inner classes bindable independently would solve this nicely. Let me think about this for a bit, maybe there is an elegant way to do that.

lyskov avatar Jun 23 '21 21:06 lyskov