binder icon indicating copy to clipboard operation
binder copied to clipboard

Remove default constructor for struct s

Open yosoufe opened this issue 6 years ago • 7 comments

Is there a way to disable generation of default constructor for the structs??

I have a abstract struct with a pure virtual copy() function. The bindings for the type should be generated but without any constructor. I added something like -function A::A (for struct A) to the config but still the constructor is created and I can create the object in python.

yosoufe avatar Apr 15 '19 23:04 yosoufe

Yes, in your C++ code please add A() =delete; into A struct definition.

lyskov avatar Apr 15 '19 23:04 lyskov

Thanks. It seems that does not work. For example I have the following abstract base struct and children structs that I need bindings for them

struct Base{
  Base() = delete;
  virtual Base* copy() const;
}
struct Child_1 : public Base{
  int member;
  Base* copy() const { /* its implementation*/}
  Child_1(){};
  Child_1(int m): member(m){};   // <<<< ----- I get compile error here if I use Base() = delete; (use of deleted function Base::Base()) 
}

struct Child_2 : public Base{
  double var;
  Base* copy() const { /* its implementation*/}
  Child_1(){};
  Child_1(double m): var(m){};   // <<<< ----- I get compile error here if I use Base() = delete; (use of deleted function Base::Base()) 
}

void do_something(Base &b)
{
  if (dynamic_cast<Child_1 *>(&b)) { /* it is Child_1 and do something with it */}
  else if (dynamic_cast<Child_2 *>(&b)) { /* it is Child_2 and do something with it */}
}

Now I want to generate binding for this. That in python you are only allowed to make object from childern but you need the base class definition to generate binding for the do_something function (because of its argument's type). And in python you should not be able to create object of Base the same as in C++.

It seems if I change the is_callback_structure_constructible function in class.cpp to just return false if isAbstract is true and without further checks, it will give my expected behavior. But I am not sure if this is a correct way of doing it. That would mean to change from

// Check if call-back structure that we can create will be constructible
bool is_callback_structure_constructible(CXXRecordDecl const *C)
{
	if( C->isAbstract() ) {
		for(auto m = C->method_begin(); m != C->method_end(); ++m) {
			if( m->isPure()  and  !isa<CXXConstructorDecl>(*m)  and  ( m->getAccess() == AS_private  or  !is_bindable(*m)  or  is_skipping_requested(*m, Config::get()) ) ) return false;
		}

		for(auto b = C->bases_begin(); b!=C->bases_end(); ++b) {
			if( auto rt = dyn_cast<RecordType>(b->getType().getCanonicalType().getTypePtr() ) ) {
				if(CXXRecordDecl *R = cast<CXXRecordDecl>(rt->getDecl()) ) {
				if( b->getAccessSpecifier() != AS_private ) {
					if( !is_callback_structure_constructible(R) ) return false;
				}
				else if( R->isAbstract() ) return false;
				}
			}
		}
	}

	return true;
}

To simply

// Check if call-back structure that we can create will be constructible
bool is_callback_structure_constructible(CXXRecordDecl const *C)
{
	if( C->isAbstract() ) {
		return false;
	}
	return true;
}

I do not know if it is going to have consequences.

yosoufe avatar Apr 15 '19 23:04 yosoufe

@lyskov What are PyCallBack_ structs that are being generated? What does the callback_structure mean in this context?

yosoufe avatar Apr 16 '19 17:04 yosoufe

@lyskov On this branch: https://github.com/yosoufe/binder/tree/yousof/dev_cosntructors You can check the differences: https://github.com/RosettaCommons/binder/compare/master...yosoufe:yousof/dev_cosntructors I have added some more configs like

-callback_struct Base
-general_callback_structs  someWord

So I can turn off generating the PyCallBack_ struct. Quick Question, Are those PyCallBack_s are required to be able to inherit from the C++ classes?? If I do not want to do that, I can turn them off?

yosoufe avatar Apr 16 '19 19:04 yosoufe

I see, so you wanted to keep constructor in C++ code but does not want it to be exposed in Python, is this correct? If so, then have you considered making your Base declared as protected? That way your derived classes will be able to call it but it should not be exposed in Python.

re callback_structure: this is a mechanism to allow creation of derived classes in Python, please see https://pybind11.readthedocs.io/en/stable/advanced/classes.html#overriding-virtual-functions-in-python for more details.

Right now there is no way to turn off trampoline generation, - why would you want that? (It does not incur any runtime overhead if no virtual function is overriden in Python)

lyskov avatar Apr 16 '19 22:04 lyskov

Thank you @lyskov I am making a C++ API that has some Base classes that the user should not make object from them. In C++ they cannot, because the base has pure virtual functions. I am generating the python bindings for the same C++ API and I am looking for the same behavior. But I need the binding for the Base class without constructor. Because some functions have arguments with reference type of that Base class. If I do not generate binding without constructor for the base, I cannot generate the python bindings for functions that have those Base class reference type because they are unknown in Python. With that branch on my repo and those configs I can do what I want. Maybe it can be also useful to others.

yosoufe avatar Apr 17 '19 00:04 yosoufe

Ah, i see: so you want to be able to have precise control generation on callback generation - i agree that it might be useful in some cases so pull-request for this will be welcome.

(Note that I am still not sure why do you want to forbid deriving from Base in the case above. Let me elaborate: Binder "design philosophy" is that in general Python code should function the same way as C++ code (when possible). So since you can derive from Base in C++ then generated bindings you should allow you to derive from it in Python as well. If however we want to alter behavior/API of final Python bindings then this should be done on level higher then raw bindings code. Common way to achieve this is to provide extra .py file for final bindings package that act as primary import point for Python users. In such Python file we can adjust raw bindings by providing extra methods (written in Python) in addition to C++ methods, deleting/enhancing methods, etc)

I just checked https://github.com/RosettaCommons/binder/compare/master...yosoufe:yousof/dev_cosntructors - other then a few cosmetic refactoring it looks good to me!

Thanks,

lyskov avatar Apr 17 '19 23:04 lyskov