shiboken6: Simplify handling of removed attribute

The meaning of the TypeSystem::Language enumeration value
on the remove elements was unclear; and it is only
ever used with 'all'. Replace it by a boolean value
and simplify the code accordingly.

Adapt the documentation.

Change-Id: I82d082e6d551403642936294a11abbac09d723dd
Reviewed-by: Christian Tismer <tismer@stackless.com>

9 files changed, 57 insertions, 90 deletions

diff --git a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
index 6704054d1..efe2f5ec8 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetabuilder.cpp
@@ -2674,7 +2674,7 @@ void AbstractMetaBuilderPrivate::inheritTemplateFunctions(AbstractMetaClass *sub
     for (const auto &function : templateClassFunctions) {
         // If the function is modified or the instantiation has an equally named
         // function we have shadowing, so we need to skip it.
-        if (function->isModifiedRemoved(TypeSystem::All)
+        if (function->isModifiedRemoved()
             || AbstractMetaFunction::find(existingSubclassFuncs, function->name()) != nullptr) {
             continue;
         }
diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
index df0ce2a1c..79ca961f6 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.cpp
@@ -193,13 +193,12 @@ AbstractMetaFunction::~AbstractMetaFunction() = default;
 /*******************************************************************************
  * Indicates that this function has a modification that removes it
  */
-bool AbstractMetaFunction::isModifiedRemoved(int types) const
+bool AbstractMetaFunction::isModifiedRemoved(const AbstractMetaClass *cls) const
 {
-    for (const auto &mod : modifications(implementingClass())) {
-        if (!mod.isRemoveModifier())
-            continue;
-
-        if ((mod.removal() & types) == types)
+    if (d->m_functionType != GlobalScopeFunction && cls == nullptr)
+        cls = d->m_implementingClass;
+    for (const auto &mod : modifications(cls)) {
+        if (mod.isRemoved())
             return true;
     }
 
@@ -634,21 +633,6 @@ TypeSystem::Ownership AbstractMetaFunction::ownership(const AbstractMetaClass *c
     return TypeSystem::InvalidOwnership;
 }
 
-bool AbstractMetaFunction::isRemovedFromAllLanguages(const AbstractMetaClass *cls) const
-{
-    return isRemovedFrom(cls, TypeSystem::All);
-}
-
-bool AbstractMetaFunction::isRemovedFrom(const AbstractMetaClass *cls, TypeSystem::Language language) const
-{
-    for (const auto &modification : modifications(cls)) {
-        if ((modification.removal() & language) == language)
-            return true;
-    }
-
-    return false;
-}
-
 QString AbstractMetaFunction::typeReplaced(int key) const
 {
     for (const auto &modification : modifications(declaringClass())) {
diff --git a/sources/shiboken6/ApiExtractor/abstractmetafunction.h b/sources/shiboken6/ApiExtractor/abstractmetafunction.h
index ff30308c1..1a29fcefe 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetafunction.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetafunction.h
@@ -158,7 +158,7 @@ public:
     QString minimalSignature() const;
     QString debugSignature() const; // including virtual/override/final, etc., for debugging only.
 
-    bool isModifiedRemoved(int types = TypeSystem::All) const;
+    bool isModifiedRemoved(const AbstractMetaClass *cls = nullptr) const;
 
     bool isVoid() const;
 
@@ -225,8 +225,7 @@ public:
 
     QString typeReplaced(int argument_index) const;
     bool isModifiedToArray(int argumentIndex) const;
-    bool isRemovedFromAllLanguages(const AbstractMetaClass *) const;
-    bool isRemovedFrom(const AbstractMetaClass *, TypeSystem::Language language) const;
+
     bool argumentRemoved(int) const;
     /**
     *   Verifies if any modification to the function is an inject code.
diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
index 52a8570e5..9fdc32d43 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
+++ b/sources/shiboken6/ApiExtractor/abstractmetalang.cpp
@@ -158,7 +158,7 @@ AbstractMetaFunctionCList AbstractMetaClass::queryFunctionsByName(const QString
  */
 AbstractMetaFunctionCList AbstractMetaClass::functionsInTargetLang() const
 {
-    FunctionQueryOptions default_flags = NormalFunctions | Visible | NotRemovedFromTargetLang;
+    FunctionQueryOptions default_flags = NormalFunctions | Visible | NotRemoved;
 
     // Only public functions in final classes
     // default_flags |= isFinal() ? WasPublic : 0;
@@ -851,14 +851,11 @@ bool AbstractMetaClass::generateExceptionHandling() const
 
 bool AbstractMetaClass::queryFunction(const AbstractMetaFunction *f, FunctionQueryOptions query)
 {
-    if ((query & NotRemovedFromTargetLang)
-        && f->isRemovedFrom(f->implementingClass(), TypeSystem::TargetLangCode)) {
-        return false;
-    }
-
-    if ((query & NotRemovedFromTargetLang) && f->isVirtual()
-        && f->isRemovedFrom(f->declaringClass(), TypeSystem::TargetLangCode)) {
-        return false;
+    if ((query.testFlag(NotRemoved))) {
+        if (f->isModifiedRemoved())
+            return false;
+        if (f->isVirtual() && f->isModifiedRemoved(f->declaringClass()))
+            return false;
     }
 
     if ((query & Visible) && f->isPrivate())
@@ -959,12 +956,12 @@ AbstractMetaFunctionCList AbstractMetaClass::queryFunctions(FunctionQueryOptions
 
 bool AbstractMetaClass::hasSignals() const
 {
-    return queryFirstFunction(d->m_functions, Signals | Visible | NotRemovedFromTargetLang) != nullptr;
+    return queryFirstFunction(d->m_functions, Signals | Visible | NotRemoved) != nullptr;
 }
 
 AbstractMetaFunctionCList AbstractMetaClass::cppSignalFunctions() const
 {
-    return queryFunctions(Signals | Visible | NotRemovedFromTargetLang);
+    return queryFunctions(Signals | Visible | NotRemoved);
 }
 
 std::optional<AbstractMetaField>
@@ -1095,7 +1092,7 @@ void AbstractMetaClass::fixFunctions()
         // Fishy: Setting up of implementing/declaring/base classes changes
         // the applicable modifications; clear cached ones.
         qSharedPointerConstCast<AbstractMetaFunction>(f)->clearModificationsCache();
-        if (!f->isRemovedFromAllLanguages(f->implementingClass()))
+        if (!f->isModifiedRemoved())
             nonRemovedFuncs.append(f);
     }
 
@@ -1117,7 +1114,7 @@ void AbstractMetaClass::fixFunctions()
 
         QSet<AbstractMetaFunctionCPtr> funcsToAdd;
         for (const auto &sf : qAsConst(superFuncs)) {
-            if (sf->isRemovedFromAllLanguages(sf->implementingClass()))
+            if (sf->isModifiedRemoved())
                 continue;
 
             // skip functions added in base classes
@@ -1262,9 +1259,7 @@ void AbstractMetaClass::fixFunctions()
 
 
         // Make sure that we include files for all classes that are in use
-
-        if (!func->isRemovedFrom(this, TypeSystem::ShellCode))
-            addExtraIncludesForFunction(this, func);
+        addExtraIncludesForFunction(this, func);
     }
 
     if (hasPrivateConstructors && !hasPublicConstructors) {
diff --git a/sources/shiboken6/ApiExtractor/abstractmetalang.h b/sources/shiboken6/ApiExtractor/abstractmetalang.h
index f1380e9da..bafd7585f 100644
--- a/sources/shiboken6/ApiExtractor/abstractmetalang.h
+++ b/sources/shiboken6/ApiExtractor/abstractmetalang.h
@@ -69,7 +69,7 @@ public:
         Invisible                    = 0x0010000, // Only private functions
         VirtualInCppFunctions        = 0x0020000, // Only functions that are virtual in C++
         VirtualInTargetLangFunctions = 0x0080000, // Only functions which are virtual in TargetLang
-        NotRemovedFromTargetLang     = 0x0400000, // Only functions that have not been removed from TargetLang
+        NotRemoved                   = 0x0400000, // Only functions that have not been removed
         OperatorOverloads            = 0x2000000, // Only functions that are operator overloads
         GenerateExceptionHandling    = 0x4000000,
         GetAttroFunction             = 0x8000000,
diff --git a/sources/shiboken6/ApiExtractor/modifications.cpp b/sources/shiboken6/ApiExtractor/modifications.cpp
index fe3983ce1..e70c7f7d8 100644
--- a/sources/shiboken6/ApiExtractor/modifications.cpp
+++ b/sources/shiboken6/ApiExtractor/modifications.cpp
@@ -459,7 +459,7 @@ class ModificationData : public QSharedData
 public:
     QString renamedToName;
     Modification::Modifiers modifiers;
-    TypeSystem::Language removal = TypeSystem::NoLanguage;
+    bool removed = false;
 };
 
 Modification::Modification() : md(new ModificationData)
@@ -475,8 +475,8 @@ Modification::~Modification() = default;
 void Modification::formatDebug(QDebug &debug) const
 {
     debug << "modifiers=" << md->modifiers;
-    if (md->removal)
-      debug << ", removal";
+    if (md->removed)
+      debug << ", removed";
     if (!md->renamedToName.isEmpty())
         debug << ", renamedToName=\"" << md->renamedToName << '"';
 }
@@ -517,15 +517,15 @@ void Modification::clearModifierFlag(ModifierFlag f)
         md->modifiers = newMods;
 }
 
-TypeSystem::Language Modification::removal() const
+bool Modification::isRemoved() const
 {
-    return md->removal;
+    return md->removed;
 }
 
-void Modification::setRemoval(TypeSystem::Language r)
+void Modification::setRemoved(bool r)
 {
-    if (md->removal != r)
-        md->removal = r;
+    if (md->removed != r)
+        md->removed = r;
 }
 
 class FunctionModificationData : public QSharedData
diff --git a/sources/shiboken6/ApiExtractor/modifications.h b/sources/shiboken6/ApiExtractor/modifications.h
index 51fd46ed8..1dc66cf74 100644
--- a/sources/shiboken6/ApiExtractor/modifications.h
+++ b/sources/shiboken6/ApiExtractor/modifications.h
@@ -258,8 +258,8 @@ public:
     void setModifiers(Modifiers m);
     void setModifierFlag(ModifierFlag f);
     void clearModifierFlag(ModifierFlag f);
-    TypeSystem::Language removal() const;
-    void setRemoval(TypeSystem::Language r);
+    bool isRemoved() const;
+    void setRemoved(bool r);
 
     bool isAccessModifier() const
     {
@@ -305,10 +305,7 @@ public:
         return modifiers().testFlag(Rename);
     }
 
-    bool isRemoveModifier() const
-    {
-        return removal() != TypeSystem::NoLanguage;
-    }
+    bool isRemoveModifier() const { return isRemoved(); }
 
 #ifndef QT_NO_DEBUG_STREAM
     void formatDebug(QDebug &d) const;
diff --git a/sources/shiboken6/ApiExtractor/typesystemparser.cpp b/sources/shiboken6/ApiExtractor/typesystemparser.cpp
index 7501a6eaf..1b7864e30 100644
--- a/sources/shiboken6/ApiExtractor/typesystemparser.cpp
+++ b/sources/shiboken6/ApiExtractor/typesystemparser.cpp
@@ -1059,28 +1059,10 @@ static bool convertBoolean(QStringView value, const QString &attributeName, bool
     return defaultValue;
 }
 
-static bool convertRemovalAttribute(QStringView remove, Modification& mod, QString& errorMsg)
+static bool convertRemovalAttribute(QStringView value)
 {
-    if (remove.isEmpty())
-        return true;
-#ifdef QTBUG_69389_FIXED
-    if (remove.compare(u"all", Qt::CaseInsensitive) == 0) {
-#else
-    if (QtPrivate::compareStrings(remove, u"all", Qt::CaseInsensitive) == 0) {
-#endif
-        mod.setRemoval(TypeSystem::All);
-        return true;
-    }
-#ifdef QTBUG_69389_FIXED
-    if (remove.compare(u"target", Qt::CaseInsensitive) == 0) {
-#else
-    if (QtPrivate::compareStrings(remove, u"target", Qt::CaseInsensitive) == 0) {
-#endif
-        mod.setRemoval(TypeSystem::TargetLangAndNativeCode);
-        return true;
-    }
-    errorMsg = QString::fromLatin1("Bad removal type '%1'").arg(remove);
-    return false;
+    return value == u"all" // Legacy
+        || convertBoolean(value, removeAttribute(), false);
 }
 
 // Check whether an entry should be dropped, allowing for dropping the module
@@ -2094,8 +2076,7 @@ bool TypeSystemParser::parseModifyField(const QXmlStreamReader &reader,
         if (name == nameAttribute()) {
             fm.setName(attributes->takeAt(i).value().toString());
         } else if (name == removeAttribute()) {
-            if (attributes->takeAt(i).value() == u"all")
-                fm.setRemoved(true);
+            fm.setRemoved(convertRemovalAttribute(attributes->takeAt(i).value()));
         }  else if (name == readAttribute()) {
             qCWarning(lcShiboken, "%s",
                       qPrintable(msgUnimplementedAttributeWarning(reader, name)));
@@ -2253,7 +2234,7 @@ bool TypeSystemParser::parseModifyFunction(const QXmlStreamReader &reader,
 
     QString originalSignature;
     QString access;
-    QString removal;
+    bool removed = false;
     QString rename;
     bool deprecated = false;
     bool isThread = false;
@@ -2269,7 +2250,7 @@ bool TypeSystemParser::parseModifyFunction(const QXmlStreamReader &reader,
         } else if (name == renameAttribute()) {
             rename = attributes->takeAt(i).value().toString();
         } else if (name == removeAttribute()) {
-            removal = attributes->takeAt(i).value().toString();
+            removed = convertRemovalAttribute(attributes->takeAt(i).value());
         } else if (name == deprecatedAttribute()) {
             deprecated = convertBoolean(attributes->takeAt(i).value(),
                                         deprecatedAttribute(), false);
@@ -2342,8 +2323,7 @@ bool TypeSystemParser::parseModifyFunction(const QXmlStreamReader &reader,
     if (deprecated)
         mod.setModifierFlag(Modification::Deprecated);
 
-    if (!removal.isEmpty() && !convertRemovalAttribute(removal, mod, m_error))
-        return false;
+    mod.setRemoved(removed);
 
     if (!rename.isEmpty()) {
         mod.setRenamedToName(rename);
diff --git a/sources/shiboken6/doc/typesystem_manipulating_objects.rst b/sources/shiboken6/doc/typesystem_manipulating_objects.rst
index 453fa6752..dcb3a46b1 100644
--- a/sources/shiboken6/doc/typesystem_manipulating_objects.rst
+++ b/sources/shiboken6/doc/typesystem_manipulating_objects.rst
@@ -77,15 +77,20 @@ modify-field
          <object-type>
              <modify-field name="..."
                  write="true | false"
-                 read="true | false" />
+                 read="true | false"
+                 remove="true | false" />
          </object-type>
 
     The ``name`` attribute is the name of the field, the *optional* ``write``
     and ``read`` attributes specify the field's access privileges in the target
     language API (both are set to true by default).
-    The ``remove`` attribute is an *optional* attribute, which can mark the field
-    to be discarded on generation; it has the same purpose of the deprecated tag
-    :ref:`remove`.
+
+    The ``remove`` attribute is an *optional* boolean attribute, which can
+    mark the field to be discarded on generation.
+
+    The  *optional* ``rename`` attribute can be used to change the name of the
+    given field in the generated target language API.
+
 
 .. _modify-function:
 
@@ -102,7 +107,7 @@ modify-function
          <object-type>
              <modify-function signature="..."
                               since="..."
-                              remove="all | c++"
+                              remove="true | false"
                               access="public | private | protected"
                               allow-thread="true | auto | false"
                               exception-handling="off | auto-off | auto-on | on"
@@ -177,8 +182,15 @@ modify-function
     Numbers should be given for all overloads; otherwise, the order will be in
     declaration order.
 
-    The ``remove``, ``access`` and ``rename`` attributes are *optional* attributes
-    for added convenience; they serve the same purpose as the deprecated tags :ref:`remove`, :ref:`access` and :ref:`rename`.
+    The ``remove`` attribute is an *optional* boolean attribute, which can
+    mark the function to be discarded on generation.
+
+    The  *optional* ``rename`` attribute can be used to change the name of the
+    given function in the generated target language API.
+
+    The  *optional* ``access`` attribute changes the access privileges of the
+    given function in the generated target language API.
+
 
 .. _add-function: