Added an option to generate code without using the "#define protected public"

hack, that doesn't work on the MS Windows platform. When generating the
bindings it adds public dispatchers for protected methods on C++ classes
that have them. When avoiding the hack classes that have protected methods
get C++ wrappers instead of just the ones with virtual methods.
On top of this, support for virtual (and protected) destructors have improved.

4 files changed, 104 insertions, 20 deletions

diff --git a/CMakeLists.txt b/CMakeLists.txt
index 95673330b..1d9682c00 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -15,6 +15,9 @@ else (MSVC)
     set(CMAKE_CXX_FLAGS_RELEASE "-Wall -DNDEBUG -O2 -Wl,-O1 -Wl,--hash-style=gnu -fvisibility=hidden")
     set(CMAKE_CXX_FLAGS_DEBUG "-g -Wall -fvisibility=hidden")
 endif (MSVC)
+if (WIN32)
+add_definitions("-DAVOID_PROTECTED_HACK")
+endif(WIN32)
 
 set(LIB_SUFFIX "" CACHE STRING "Define suffix of directory name (32/64)" )
 set(LIB_INSTALL_DIR "${CMAKE_INSTALL_PREFIX}/lib${LIB_SUFFIX}" CACHE PATH "The subdirectory relative to the install prefix where libraries will be installed (default is /lib${LIB_SUFFIX})" FORCE)
diff --git a/cppgenerator.cpp b/cppgenerator.cpp
index 3449441d7..d85a0f4c8 100644
--- a/cppgenerator.cpp
+++ b/cppgenerator.cpp
@@ -79,11 +79,12 @@ void CppGenerator::generateClass(QTextStream &s, const AbstractMetaClass *metaCl
     // write license comment
     s << licenseComment() << endl;
 
+#ifndef AVOID_PROTECTED_HACK
     if (!metaClass->isNamespace() && !metaClass->hasPrivateDestructor()) {
-        //workaround to access protected functions
         s << "//workaround to access protected functions" << endl;
         s << "#define protected public" << endl << endl;
     }
+#endif
 
     // headers
     s << "// default includes" << endl;
@@ -165,7 +166,7 @@ void CppGenerator::generateClass(QTextStream &s, const AbstractMetaClass *metaCl
         if (overloads.isEmpty())
             continue;
 
-        const AbstractMetaFunction* rfunc = overloads[0];
+        const AbstractMetaFunction* rfunc = overloads.first();
         if (m_sequenceProtocol.contains(rfunc->name()))
             continue;
 
@@ -423,7 +424,9 @@ void CppGenerator::writeConstructorWrapper(QTextStream& s, const AbstractMetaFun
     s << '{' << endl;
 
     s << INDENT << "PyObject* self;" << endl;
-    s << INDENT << getFunctionReturnType(rfunc) << " cptr;" << endl << endl;
+    s << INDENT;
+    s << (shouldGenerateCppWrapper(rfunc->ownerClass()) ? wrapperName(rfunc->ownerClass()) : rfunc->ownerClass()->qualifiedCppName());
+    s << "* cptr;" << endl << endl;
 
     if (rfunc->ownerClass()->isAbstract()) {
         s << INDENT << "if (type == &" << className << ") {" << endl;
@@ -923,7 +926,15 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f
     if (func->hasInjectedCode()) {
         snips = getCodeSnips(func);
         if (injectedCodeUsesCppSelf(func)) {
-            s << INDENT << func->ownerClass()->name() << "* cppSelf = ";
+            s << INDENT;
+#ifdef AVOID_PROTECTED_HACK
+            bool hasProtectedFunctions = func->ownerClass()->hasProtectedFunctions();
+            QString _wrapperName = wrapperName(func->ownerClass());
+            s << (hasProtectedFunctions ? _wrapperName : func->ownerClass()->qualifiedCppName()) << "* cppSelf = ";
+            s << (hasProtectedFunctions ? QString("(%1*)").arg(_wrapperName) : "");
+#else
+            s << func->ownerClass()->qualifiedCppName() << "* cppSelf = ";
+#endif
             s << cpythonWrapperCPtr(func->ownerClass(), "self") << ';' << endl;
         }
 
@@ -1055,11 +1066,24 @@ void CppGenerator::writeMethodCall(QTextStream& s, const AbstractMetaFunction* f
                 if (func->type())
                     s << retvalVariableName() << " = ";
                 if (func->ownerClass()) {
+#ifndef AVOID_PROTECTED_HACK
                     if (!func->isStatic())
                         mc << cpythonWrapperCPtr(func->ownerClass()) << "->";
-                    mc << func->ownerClass()->name() << "::";
+                    mc << func->ownerClass()->name() << "::" << func->originalName();
+#else
+                    if (!func->isStatic()) {
+                        if (func->isProtected())
+                            mc << "((" << wrapperName(func->ownerClass()) << "*)";
+                        mc << cpythonWrapperCPtr(func->ownerClass());
+                        mc << (func->isProtected() ? ")" : "") << "->";
+                    }
+                    mc << (func->isProtected() ? wrapperName(func->ownerClass()) : func->ownerClass()->name());
+                    mc << "::" << func->originalName() << (func->isProtected() ? "_protected" : "");
+#endif
+                } else {
+                    mc << func->originalName();
                 }
-                mc << func->originalName() << '(' << userArgs.join(", ") << ')';
+                mc << '(' << userArgs.join(", ") << ')';
             }
         }
 
@@ -1160,7 +1184,13 @@ void CppGenerator::writeClassDefinition(QTextStream& s, const AbstractMetaClass*
         tp_new = "0";
     } else {
         tp_flags = "Py_TPFLAGS_DEFAULT|Py_TPFLAGS_BASETYPE|Py_TPFLAGS_CHECKTYPES";
-        tp_dealloc = QString("(destructor)&(Shiboken::PyBaseWrapper_Dealloc< %1 >)").arg(cppClassName);
+
+        QString deallocClassName;
+        if (shouldGenerateCppWrapper(metaClass))
+            deallocClassName = wrapperName(metaClass);
+        else
+            deallocClassName = cppClassName;
+        tp_dealloc = QString("(destructor)&(Shiboken::PyBaseWrapper_Dealloc< %1 >)").arg(deallocClassName);
 
         AbstractMetaFunctionList ctors = metaClass->queryFunctions(AbstractMetaClass::Constructors);
         tp_new = ctors.isEmpty() ? "0" : className + "_New";
@@ -1246,7 +1276,20 @@ void CppGenerator::writeSequenceMethods(QTextStream& s, const AbstractMetaClass*
         CodeSnipList snips = func->injectedCodeSnips(CodeSnip::Any, TypeSystem::TargetLangCode);
         s << funcRetVal << ' ' << funcName << '(' << funcArgs << ')' << endl << '{' << endl;
         writeInvalidCppObjectCheck(s);
-        s << INDENT << func->ownerClass()->name() << "* cppSelf = ";
+        s << INDENT;
+
+#ifndef AVOID_PROTECTED_HACK
+        s << func->ownerClass()->name() << "* cppSelf = ";
+#else
+        if (func->isProtected())
+            s << wrapperName(func->ownerClass());
+        else
+            s << func->ownerClass()->name();
+        s << "* cppSelf = ";
+        if (func->isProtected())
+            s << '(' << wrapperName(func->ownerClass()) << "*) ";
+#endif
+
         s << cpythonWrapperCPtr(func->ownerClass(), "self") << ';' << endl;
         s << INDENT << "(void)cppSelf; // avoid warnings about unused variables" << endl;
         writeCodeSnips(s, snips,CodeSnip::Any, TypeSystem::TargetLangCode, func);
diff --git a/headergenerator.cpp b/headergenerator.cpp
index b31b92e61..9074726fd 100644
--- a/headergenerator.cpp
+++ b/headergenerator.cpp
@@ -59,9 +59,9 @@ void HeaderGenerator::generateClass(QTextStream& s, const AbstractMetaClass* met
     s << "#ifndef SBK_" << wrapperName.toUpper() << "_H" << endl;
     s << "#define SBK_" << wrapperName.toUpper() << "_H" << endl<< endl;
 
-    if (shouldGenerateCppWrapper(metaClass)) {
-        s << "#define protected public" << endl << endl;
-    }
+#ifndef AVOID_PROTECTED_HACK
+    s << "#define protected public" << endl << endl;
+#endif
 
     s << "#include <shiboken.h>" << endl << endl;
 
@@ -69,9 +69,6 @@ void HeaderGenerator::generateClass(QTextStream& s, const AbstractMetaClass* met
     if (metaClass->typeEntry()->include().isValid())
         s << metaClass->typeEntry()->include().toString() << endl << endl;
 
-    writeCodeSnips(s, metaClass->typeEntry()->codeSnips(),
-                   CodeSnip::Declaration, TypeSystem::NativeCode);
-
     if (shouldGenerateCppWrapper(metaClass)) {
         // Class
         s << "class " << wrapperName;
@@ -86,7 +83,9 @@ void HeaderGenerator::generateClass(QTextStream& s, const AbstractMetaClass* met
             writeFunction(s, func);
 
         //destructor
-        s << INDENT << "~" << wrapperName << "();" << endl;
+        s << INDENT << (metaClass->hasVirtualDestructor() ? "virtual " : "") << "~" << wrapperName << "();" << endl;
+
+        writeCodeSnips(s, metaClass->typeEntry()->codeSnips(), CodeSnip::Declaration, TypeSystem::NativeCode);
 
         if (metaClass->isQObject() && (metaClass->name() != "QObject"))
             s << INDENT << "using QObject::parent;" << endl;
@@ -99,14 +98,24 @@ void HeaderGenerator::generateClass(QTextStream& s, const AbstractMetaClass* met
 
 void HeaderGenerator::writeFunction(QTextStream& s, const AbstractMetaFunction* func) const
 {
-    // pure virtual functions need a default implementation
-    if (func->isPrivate() || (func->isModifiedRemoved() && !func->isAbstract()))
-        return;
-
     // do not write copy ctors here.
     if (func->isCopyConstructor())
         return;
 
+#ifdef AVOID_PROTECTED_HACK
+    if (func->isProtected() && !func->isConstructor()) {
+        s << INDENT << "inline " << (func->isStatic() ? "static " : "");
+        s << functionSignature(func, "", "_protected") << " { ";
+        s << (func->type() ? "return " : "") << func->ownerClass()->qualifiedCppName() << "::";
+        writeFunctionCall(s, func);
+        s << "; }" << endl;
+    }
+#endif
+
+    // pure virtual functions need a default implementation
+    if (func->isPrivate() || (func->isModifiedRemoved() && !func->isAbstract()))
+        return;
+
     if (func->isConstructor() || func->isAbstract() || func->isVirtual()) {
         s << INDENT;
         if (func->isVirtual() || func->isAbstract())
diff --git a/shibokengenerator.cpp b/shibokengenerator.cpp
index b40598579..58a3bf518 100644
--- a/shibokengenerator.cpp
+++ b/shibokengenerator.cpp
@@ -182,7 +182,11 @@ QString ShibokenGenerator::translateTypeForWrapperMethod(const AbstractMetaType*
 
 bool ShibokenGenerator::shouldGenerateCppWrapper(const AbstractMetaClass* metaClass)
 {
-    return metaClass->isPolymorphic() && !metaClass->isNamespace() && !metaClass->hasPrivateDestructor();
+    bool result = metaClass->isPolymorphic() || metaClass->hasVirtualDestructor();
+#ifdef AVOID_PROTECTED_HACK
+    result = result || metaClass->hasProtectedFunctions() || metaClass->hasProtectedDestructor();
+#endif
+    return result && !metaClass->isNamespace() && !metaClass->hasPrivateDestructor();
 }
 
 QString ShibokenGenerator::wrapperName(const AbstractMetaClass* metaClass)
@@ -767,7 +771,11 @@ AbstractMetaFunctionList ShibokenGenerator::filterFunctions(const AbstractMetaCl
         //skip signals
         if (func->isSignal()
             || func->isDestructor()
+#ifndef AVOID_PROTECTED_HACK
             || (func->isModifiedRemoved() && !func->isAbstract()))
+#else
+            || (func->isModifiedRemoved() && !func->isAbstract() && !func->isProtected()))
+#endif
             lst.removeOne(func);
     }
 
@@ -976,6 +984,27 @@ void ShibokenGenerator::writeCodeSnips(QTextStream& s,
                 code.replace("%PYTHON_METHOD_OVERRIDE", "py_override");
             }
 
+#ifdef AVOID_PROTECTED_HACK
+            // If the function being processed was added by the user via type system,
+            // Shiboken needs to find out if there are other overloads for the same method
+            // name and if any of them is of the protected visibility. This is used to replace
+            // calls to %FUNCTION_NAME on user written custom code for calls to the protected
+            // dispatcher.
+            bool hasProtectedOverload = false;
+            if (func->functionType() == AbstractMetaFunction::UserAddedFunction) {
+                foreach (const AbstractMetaFunction* f, getFunctionOverloads(func->ownerClass(), func->name()))
+                    hasProtectedOverload |= f->isProtected();
+            }
+
+            if (func->isProtected() || hasProtectedOverload) {
+                code.replace("%TYPE::%FUNCTION_NAME",
+                             QString("%1::%2_protected")
+                             .arg(wrapperName(func->ownerClass()))
+                             .arg(func->originalName()));
+                code.replace("%FUNCTION_NAME", QString("%1_protected").arg(func->originalName()));
+            }
+#endif
+
             replaceTemplateVariables(code, func);
         }