From 69d5373131f35b57d7b550ed7f50fd775ad7aa34 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Wed, 2 Aug 2023 08:48:34 +0100 Subject: [PATCH] Fix missing constructor generation due to abstract class test failure when a method is declared in the class along with a using declaration and the using declaration is declared before the method that implemented the pure virtual method, such as: struct ConcreteDerived : AbstractBase { ConcreteDerived() {} // was not wrapped using AbstractBase::f; virtual void f(int n) override {} }; SourceForge bug: https://sourceforge.net/p/swig/bugs/932/ check_implemented in allocate.cxx was correctly finding a non-abstract method, however, Swig_symbol_clookup_local_check, was using the using declaration node instead of using the node returned by check_implemented. The checkfunc is now given more control by returning the node to use rather than Swig_symbol_clookup_local_check always using the head of the csym linked list. --- CHANGES.current | 13 +++++ Examples/test-suite/abstract_inherit_using.i | 31 +++++++++++ .../java/abstract_inherit_using_runme.java | 24 +++++++++ Source/Modules/allocate.cxx | 14 ++--- Source/Modules/guile.cxx | 7 ++- Source/Swig/swig.h | 4 +- Source/Swig/symbol.c | 52 +++++++++---------- 7 files changed, 105 insertions(+), 40 deletions(-) create mode 100644 Examples/test-suite/abstract_inherit_using.i create mode 100644 Examples/test-suite/java/abstract_inherit_using_runme.java diff --git a/CHANGES.current b/CHANGES.current index bf0de1d36..cae382837 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,19 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.2.0 (in progress) =========================== +2023-08-02: wsfulton + https://sourceforge.net/p/swig/bugs/932/ + Fix missing constructor generation due to abstract class test + failure when a method is declared in the class along with a + using declaration and the using declaration is declared before + the method that implemented the pure virtual method, such as: + + struct ConcreteDerived : AbstractBase { + ConcreteDerived() {} // was not wrapped + using AbstractBase::f; + virtual void f(int n) override {} + }; + 2023-08-02: olly [PHP] Implement overloading between different integer types and between double and float. diff --git a/Examples/test-suite/abstract_inherit_using.i b/Examples/test-suite/abstract_inherit_using.i new file mode 100644 index 000000000..25c37ec39 --- /dev/null +++ b/Examples/test-suite/abstract_inherit_using.i @@ -0,0 +1,31 @@ +%module abstract_inherit_using + +%inline %{ +class AbstractBase +{ +public: + virtual void f(int n) = 0; + void f(const char *another_representation_of_n) {} + virtual ~AbstractBase() {} +}; + +class ConcreteDerived1 : public AbstractBase +{ +public: + ConcreteDerived1() {} + + // Abstract test always worked + virtual void f(int n) {} + using AbstractBase::f; +}; + +class ConcreteDerived2 : public AbstractBase +{ +public: + ConcreteDerived2() {} + + // SWIG thought this class was abstract when using declaration was before method f and didn't generate constructor + using AbstractBase::f; + virtual void f(int n) {} +}; +%} diff --git a/Examples/test-suite/java/abstract_inherit_using_runme.java b/Examples/test-suite/java/abstract_inherit_using_runme.java new file mode 100644 index 000000000..e4ec0bcef --- /dev/null +++ b/Examples/test-suite/java/abstract_inherit_using_runme.java @@ -0,0 +1,24 @@ + +import abstract_inherit_using.*; + +public class abstract_inherit_using_runme { + + static { + try { + System.loadLibrary("abstract_inherit_using"); + } catch (UnsatisfiedLinkError e) { + System.err.println("Native code library failed to load. See the chapter on Dynamic Linking Problems in the SWIG Java documentation for help.\n" + e); + System.exit(1); + } + } + + public static void main(String argv[]) + { + ConcreteDerived1 cd1 = new ConcreteDerived1(); + cd1.f(1234); + cd1.f("one"); + ConcreteDerived2 cd2 = new ConcreteDerived2(); + cd2.f(1234); + cd2.f("one"); + } +} diff --git a/Source/Modules/allocate.cxx b/Source/Modules/allocate.cxx index 03187703d..9d757c0f2 100644 --- a/Source/Modules/allocate.cxx +++ b/Source/Modules/allocate.cxx @@ -8,7 +8,12 @@ * * allocate.cxx * - * This module tries to figure out which classes and structures support + * This module also has two main purposes modifying the parse tree. + * + * First, it is responsible for adding in using declarations from base class + * members into the parse tree. + * + * Second, after each class declaration, it analyses if the class/struct supports * default constructors and destructors in C++. There are several rules that * define this behavior including pure abstract methods, private sections, * and non-default constructors in base classes. See the ARM or @@ -17,9 +22,6 @@ * Once the analysis is complete, the non-explicit/implied default constructors * and destructors are added to the parse tree. Implied copy constructors are * added too if requested via the copyctor feature. - * - * This module also is responsible for adding in using declarations from base - * class members into the parse tree. * ----------------------------------------------------------------------------- */ #include "swigmod.h" @@ -37,7 +39,7 @@ void Wrapper_virtual_elimination_mode_set(int flag) { extern "C" { static String *search_decl = 0; /* Declarator being searched */ - static int check_implemented(Node *n) { + static Node *check_implemented(Node *n) { String *decl; if (!n) return 0; @@ -51,7 +53,7 @@ extern "C" { if (!GetFlag(n, "abstract")) { Delete(decl1); Delete(decl2); - return 1; + return n; } } Delete(decl1); diff --git a/Source/Modules/guile.cxx b/Source/Modules/guile.cxx index 3946d1cb2..13e0a1172 100644 --- a/Source/Modules/guile.cxx +++ b/Source/Modules/guile.cxx @@ -105,8 +105,8 @@ static int exportprimitive = 0; // -exportprimitive argument static String *memberfunction_name = 0; extern "C" { - static int has_classname(Node *class_node) { - return Getattr(class_node, "guile:goopsclassname") ? 1 : 0; + static Node *has_classname(Node *class_node) { + return Getattr(class_node, "guile:goopsclassname") ? class_node : 0; } } @@ -732,8 +732,7 @@ public: if (goops) { if (i < numreq) { if (strcmp("void", Char(pt)) != 0) { - Node *class_node = Swig_symbol_clookup_check(pb, Getattr(n, "sym:symtab"), - has_classname); + Node *class_node = Swig_symbol_clookup_check(pb, Getattr(n, "sym:symtab"), has_classname); String *goopsclassname = !class_node ? NULL : Getattr(class_node, "guile:goopsclassname"); /* do input conversion */ if (goopsclassname) { diff --git a/Source/Swig/swig.h b/Source/Swig/swig.h index 99d265e32..301a7f09e 100644 --- a/Source/Swig/swig.h +++ b/Source/Swig/swig.h @@ -234,11 +234,11 @@ extern "C" { extern void Swig_symbol_conflict_warn(Node *n, Node *c, const String *symname, int inclass); extern void Swig_symbol_cadd(const_String_or_char_ptr symname, Node *n); extern Node *Swig_symbol_clookup(const_String_or_char_ptr symname, Symtab *tab); - extern Node *Swig_symbol_clookup_check(const_String_or_char_ptr symname, Symtab *tab, int (*check) (Node *)); + extern Node *Swig_symbol_clookup_check(const_String_or_char_ptr symname, Symtab *tab, Node *(*checkfunc) (Node *)); extern Node *Swig_symbol_clookup_no_inherit(const_String_or_char_ptr name, Symtab *n); extern Symtab *Swig_symbol_cscope(const_String_or_char_ptr symname, Symtab *tab); extern Node *Swig_symbol_clookup_local(const_String_or_char_ptr symname, Symtab *tab); - extern Node *Swig_symbol_clookup_local_check(const_String_or_char_ptr symname, Symtab *tab, int (*check) (Node *)); + extern Node *Swig_symbol_clookup_local_check(const_String_or_char_ptr symname, Symtab *tab, Node *(*checkfunc) (Node *)); extern String *Swig_symbol_qualified(Node *n); extern Node *Swig_symbol_isoverloaded(Node *n); extern void Swig_symbol_remove(Node *n); diff --git a/Source/Swig/symbol.c b/Source/Swig/symbol.c index aaa992495..66db026c5 100644 --- a/Source/Swig/symbol.c +++ b/Source/Swig/symbol.c @@ -495,7 +495,7 @@ void Swig_symbol_alias(const_String_or_char_ptr aliasname, Symtab *s) { * * Inherit symbols from another scope. Primarily for C++ inheritance and * for using directives, such as 'using namespace X;' - * but not for using declarations, such as 'using A;'. + * but not for using declarations, such as 'using X::A;'. * ----------------------------------------------------------------------------- */ void Swig_symbol_inherit(Symtab *s) { @@ -1019,12 +1019,12 @@ void Swig_symbol_conflict_warn(Node *n, Node *c, const String *symname, int incl * * This function operates in the C namespace, not the target namespace. * - * The check function is an optional callback that can be used to verify a particular + * The checkfunc function is an optional callback that can be used to verify a particular * symbol match. This is only used in some of the more exotic parts of SWIG. For instance, * verifying that a class hierarchy implements all pure virtual methods. * ----------------------------------------------------------------------------- */ -static Node *_symbol_lookup(const String *name, Symtab *symtab, int (*check) (Node *n)) { +static Node *_symbol_lookup(const String *name, Symtab *symtab, Node *(*checkfunc) (Node *n)) { Node *n; List *inherit; Hash *sym = Getattr(symtab, "csymtab"); @@ -1039,17 +1039,13 @@ static Node *_symbol_lookup(const String *name, Symtab *symtab, int (*check) (No #endif if (n) { - /* if a check-function is defined. Call it to determine a match */ - if (check) { - int c = check(n); - if (c == 1) { + /* if checkfunc is defined. Call it to determine a match */ + if (checkfunc) { + Node *cn = checkfunc(n); + if (cn) { Setmark(symtab, 0); - return n; - } - if (c < 0) { - /* Terminate the search right away */ - Setmark(symtab, 0); - return 0; + /* Note that checkfunc can return n != cn, where cn could be a node further down the csym linked list starting at n */ + return cn; } } else { Setmark(symtab, 0); @@ -1062,7 +1058,7 @@ static Node *_symbol_lookup(const String *name, Symtab *symtab, int (*check) (No Setmark(symtab, 0); dname = Swig_symbol_template_deftype(name, symtab); if (!Equal(dname, name)) { - n = _symbol_lookup(dname, symtab, check); + n = _symbol_lookup(dname, symtab, checkfunc); } Delete(dname); if (n) @@ -1075,7 +1071,7 @@ static Node *_symbol_lookup(const String *name, Symtab *symtab, int (*check) (No int i, len; len = Len(inherit); for (i = 0; i < len; i++) { - n = _symbol_lookup(name, Getitem(inherit, i), check); + n = _symbol_lookup(name, Getitem(inherit, i), checkfunc); if (n) { Setmark(symtab, 0); return n; @@ -1087,13 +1083,13 @@ static Node *_symbol_lookup(const String *name, Symtab *symtab, int (*check) (No return 0; } -static Node *symbol_lookup(const_String_or_char_ptr name, Symtab *symtab, int (*check) (Node *n)) { +static Node *symbol_lookup(const_String_or_char_ptr name, Symtab *symtab, Node *(*checkfunc) (Node *n)) { Node *n = 0; if (DohCheck(name)) { - n = _symbol_lookup(name, symtab, check); + n = _symbol_lookup(name, symtab, checkfunc); } else { String *sname = NewString(name); - n = _symbol_lookup(sname, symtab, check); + n = _symbol_lookup(sname, symtab, checkfunc); Delete(sname); } return n; @@ -1105,7 +1101,7 @@ static Node *symbol_lookup(const_String_or_char_ptr name, Symtab *symtab, int (* * symbol_lookup_qualified() * ----------------------------------------------------------------------------- */ -static Node *symbol_lookup_qualified(const_String_or_char_ptr name, Symtab *symtab, const String *prefix, int local, int (*checkfunc) (Node *n)) { +static Node *symbol_lookup_qualified(const_String_or_char_ptr name, Symtab *symtab, const String *prefix, int local, Node *(*checkfunc) (Node *n)) { /* This is a little funky, we search by fully qualified names */ if (!symtab) @@ -1269,7 +1265,7 @@ Node *Swig_symbol_clookup(const_String_or_char_ptr name, Symtab *n) { * inheritance hierarchy. * ----------------------------------------------------------------------------- */ -Node *Swig_symbol_clookup_check(const_String_or_char_ptr name, Symtab *n, int (*checkfunc) (Node *n)) { +Node *Swig_symbol_clookup_check(const_String_or_char_ptr name, Symtab *n, Node *(*checkfunc) (Node *n)) { Hash *hsym = 0; Node *s = 0; @@ -1321,8 +1317,7 @@ Node *Swig_symbol_clookup_check(const_String_or_char_ptr name, Symtab *n, int (* } /* Check if s is a 'using' node */ while (s && Checkattr(s, "nodeType", "using")) { - Node *ss; - ss = Swig_symbol_clookup(Getattr(s, "uname"), Getattr(s, "sym:symtab")); + Node *ss = Swig_symbol_clookup(Getattr(s, "uname"), Getattr(s, "sym:symtab")); if (!ss && !checkfunc) { SWIG_WARN_NODE_BEGIN(s); Swig_warning(WARN_PARSE_USING_UNDEF, Getfile(s), Getline(s), "Nothing known about '%s'.\n", SwigType_namestr(Getattr(s, "uname"))); @@ -1390,7 +1385,7 @@ Node *Swig_symbol_clookup_local(const_String_or_char_ptr name, Symtab *n) { * Swig_symbol_clookup_local_check() * ----------------------------------------------------------------------------- */ -Node *Swig_symbol_clookup_local_check(const_String_or_char_ptr name, Symtab *n, int (*checkfunc) (Node *)) { +Node *Swig_symbol_clookup_local_check(const_String_or_char_ptr name, Symtab *n, Node *(*checkfunc) (Node *)) { Hash *hsym; Node *s = 0; @@ -1439,7 +1434,8 @@ Node *Swig_symbol_clookup_local_check(const_String_or_char_ptr name, Symtab *n, /* ----------------------------------------------------------------------------- * Swig_symbol_clookup_no_inherit() * - * Symbol lookup like Swig_symbol_clookup but does not follow using declarations. + * Symbol lookup like Swig_symbol_clookup but does not follow using directives. + * Using declarations are followed. * ----------------------------------------------------------------------------- */ Node *Swig_symbol_clookup_no_inherit(const_String_or_char_ptr name, Symtab *n) { @@ -1662,12 +1658,12 @@ static SwigType *symbol_template_qualify(const SwigType *e, Symtab *st) { } -static int symbol_no_constructor(Node *n) { - return !Checkattr(n, "nodeType", "constructor"); +static Node *symbol_no_constructor(Node *n) { + return Checkattr(n, "nodeType", "constructor") ? 0 : n; } -static int symbol_is_template(Node *n) { - return Checkattr(n, "nodeType", "template"); +static Node *symbol_is_template(Node *n) { + return Checkattr(n, "nodeType", "template") ? n : 0; } /* -----------------------------------------------------------------------------