From 49cd031f12b8a98382a7ef497160bc0b2cb8512a Mon Sep 17 00:00:00 2001 From: Frank Schlimbach Date: Mon, 10 Oct 2016 10:42:23 -0500 Subject: [PATCH 1/7] restricting chaching template default types --- Source/Swig/symbol.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/Source/Swig/symbol.c b/Source/Swig/symbol.c index d72451a14..d9fc6c08a 100644 --- a/Source/Swig/symbol.c +++ b/Source/Swig/symbol.c @@ -518,7 +518,6 @@ void Swig_symbol_inherit(Symtab *s) { void Swig_symbol_cadd(const_String_or_char_ptr name, Node *n) { Node *append = 0; - Node *cn; /* There are a few options for weak symbols. A "weak" symbol is any symbol that can be replaced by another symbol in the C symbol @@ -1908,8 +1907,8 @@ SwigType *Swig_symbol_template_deftype(const SwigType *type, Symtab *tscope) { int i; #ifdef SWIG_TEMPLATE_DEFTYPE_CACHE static Hash *deftype_cache = 0; - String *scopetype = tscope ? NewStringf("%s::%s", Getattr(tscope, "name"), type) - : NewStringf("%s::%s", Swig_symbol_getscopename(), type); + String *scopetype = tscope ? NewStringf("%s::%s", Swig_symbol_qualifiedscopename(tscope), type) + : NewStringf("%s::%s", Swig_symbol_qualifiedscopename(current_symtab), type); if (!deftype_cache) { deftype_cache = NewHash(); } @@ -1918,6 +1917,9 @@ SwigType *Swig_symbol_template_deftype(const SwigType *type, Symtab *tscope) { if (cres) { Append(result, cres); Delete(scopetype); +#ifdef SWIG_DEBUG + Printf(stderr, "found cached deftype %s -> %s\n", type, result); +#endif return result; } } From e42f575b12c3462e67b66b0a8a8b67e50956b57d Mon Sep 17 00:00:00 2001 From: Frank Schlimbach Date: Mon, 10 Oct 2016 13:50:09 -0500 Subject: [PATCH 2/7] adding test --- Examples/test-suite/amodel.i | 10 +++++++ Examples/test-suite/common.mk | 1 + Examples/test-suite/template_default_cache.i | 28 ++++++++++++++++++++ 3 files changed, 39 insertions(+) create mode 100644 Examples/test-suite/amodel.i create mode 100644 Examples/test-suite/template_default_cache.i diff --git a/Examples/test-suite/amodel.i b/Examples/test-suite/amodel.i new file mode 100644 index 000000000..efb394328 --- /dev/null +++ b/Examples/test-suite/amodel.i @@ -0,0 +1,10 @@ +%module amodel; + +%inline %{ +namespace ns__a { + namespace iface1 { + class Model {}; + typedef d::d ModelPtr; + } +} + %} diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index 6e4034cb1..d77c43751 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -407,6 +407,7 @@ CPP_TEST_CASES += \ template_default_arg_overloaded \ template_default_arg_overloaded_extend \ template_default_arg_virtual_destructor \ + template_default_cache \ template_default_class_parms \ template_default_class_parms_typedef \ template_default_inherit \ diff --git a/Examples/test-suite/template_default_cache.i b/Examples/test-suite/template_default_cache.i new file mode 100644 index 000000000..5c752ae30 --- /dev/null +++ b/Examples/test-suite/template_default_cache.i @@ -0,0 +1,28 @@ +%module template_default_cache; + +%import "amodel.i" + +%{ // C++ code to make this compilable + namespace d { + template< typename T > class d {}; + } + namespace ns__a { + namespace iface1 { + class Model {}; + typedef d::d ModelPtr; + } + using iface1::ModelPtr; + } +%} + +%inline %{ +namespace ns__b { + namespace iface1 { + class Model { + public: + ns__a::ModelPtr foo() { return ns__a::ModelPtr(); }; + }; + typedef d::d ModelPtr; + } + } +%} From 4eb7fb8123aef2012ecad44b5f6961e4005c9093 Mon Sep 17 00:00:00 2001 From: Frank Schlimbach Date: Tue, 11 Oct 2016 07:05:44 -0500 Subject: [PATCH 3/7] account for explicitly qualified scopes --- Source/Swig/symbol.c | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/Source/Swig/symbol.c b/Source/Swig/symbol.c index d9fc6c08a..84a6f4860 100644 --- a/Source/Swig/symbol.c +++ b/Source/Swig/symbol.c @@ -1907,8 +1907,12 @@ SwigType *Swig_symbol_template_deftype(const SwigType *type, Symtab *tscope) { int i; #ifdef SWIG_TEMPLATE_DEFTYPE_CACHE static Hash *deftype_cache = 0; - String *scopetype = tscope ? NewStringf("%s::%s", Swig_symbol_qualifiedscopename(tscope), type) - : NewStringf("%s::%s", Swig_symbol_qualifiedscopename(current_symtab), type); + /* The lookup depends on the current scope and potential namespace qualification. + Looking up x in namespace y is not the same as looking up x::y in outer scope. + -> we contruct an "artificial" key. */ + String *scopetype = tscope + ? NewStringf("%s..%s::%s", Swig_symbol_qualifiedscopename(tscope), Getattr(tscope, "name"), type) + : NewStringf("%s..%s::%s", Swig_symbol_qualifiedscopename(current_symtab), Swig_symbol_getscopename(), type); if (!deftype_cache) { deftype_cache = NewHash(); } @@ -1916,10 +1920,10 @@ SwigType *Swig_symbol_template_deftype(const SwigType *type, Symtab *tscope) { String *cres = Getattr(deftype_cache, scopetype); if (cres) { Append(result, cres); - Delete(scopetype); #ifdef SWIG_DEBUG - Printf(stderr, "found cached deftype %s -> %s\n", type, result); + Printf(stderr, "cached deftype %s(%s) -> %s\n", type, scopetype, result); #endif + Delete(scopetype); return result; } } From be92482e27cc0d3c2513974ed044ed03b2775e34 Mon Sep 17 00:00:00 2001 From: Frank Schlimbach Date: Mon, 9 Jan 2017 09:46:33 -0600 Subject: [PATCH 4/7] using 2-level caching as suggested by @wsfulton --- Examples/test-suite/template_default_cache.i | 23 ++++++++---- Source/Swig/symbol.c | 37 +++++++++++++------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/Examples/test-suite/template_default_cache.i b/Examples/test-suite/template_default_cache.i index 5c752ae30..fd38a441d 100644 --- a/Examples/test-suite/template_default_cache.i +++ b/Examples/test-suite/template_default_cache.i @@ -2,17 +2,22 @@ %import "amodel.i" -%{ // C++ code to make this compilable +%inline %{ namespace d { template< typename T > class d {}; } - namespace ns__a { - namespace iface1 { - class Model {}; - typedef d::d ModelPtr; - } - using iface1::ModelPtr; +%} + +%ignore ns__a::iface1::Model; + +%inline %{ +namespace ns__a { + namespace iface1 { + class Model {}; + typedef d::d ModelPtr; } + using iface1::ModelPtr; +} %} %inline %{ @@ -23,6 +28,10 @@ namespace ns__b { ns__a::ModelPtr foo() { return ns__a::ModelPtr(); }; }; typedef d::d ModelPtr; + ns__a::ModelPtr get_mp_a() { return ns__a::ModelPtr(); } + ModelPtr get_mp_b() { return ModelPtr(); } } } %} +%template(AModelPtr) d::d; +%template(BModelPtr) d::d; diff --git a/Source/Swig/symbol.c b/Source/Swig/symbol.c index 84a6f4860..c548a0670 100644 --- a/Source/Swig/symbol.c +++ b/Source/Swig/symbol.c @@ -1906,26 +1906,37 @@ SwigType *Swig_symbol_template_deftype(const SwigType *type, Symtab *tscope) { int len = Len(elements); int i; #ifdef SWIG_TEMPLATE_DEFTYPE_CACHE - static Hash *deftype_cache = 0; + static Hash *s_cache = 0; + Hash *scope_cache; /* The lookup depends on the current scope and potential namespace qualification. Looking up x in namespace y is not the same as looking up x::y in outer scope. - -> we contruct an "artificial" key. */ - String *scopetype = tscope - ? NewStringf("%s..%s::%s", Swig_symbol_qualifiedscopename(tscope), Getattr(tscope, "name"), type) - : NewStringf("%s..%s::%s", Swig_symbol_qualifiedscopename(current_symtab), Swig_symbol_getscopename(), type); - if (!deftype_cache) { - deftype_cache = NewHash(); + -> we use a 2-level hash: first scope and then symbol. */ + String *scope_name = tscope + ? Swig_symbol_qualifiedscopename(tscope) + : Swig_symbol_qualifiedscopename(current_symtab); + String *type_name = tscope + ? NewStringf("%s::%s", Getattr(tscope, "name"), type) + : NewStringf("%s::%s", Swig_symbol_getscopename(), type); + if (!scope_name) scope_name = NewString("::"); + if (!s_cache) { + s_cache = NewHash(); } - if (scopetype) { - String *cres = Getattr(deftype_cache, scopetype); + scope_cache = Getattr(s_cache, scope_name); + if (scope_cache) { + String *cres = Getattr(scope_cache, type_name); if (cres) { Append(result, cres); #ifdef SWIG_DEBUG - Printf(stderr, "cached deftype %s(%s) -> %s\n", type, scopetype, result); + Printf(stderr, "cached deftype %s(%s) -> %s\n", type, scope_name, result); #endif - Delete(scopetype); + Delete(type_name); + Delete(scope_name); return result; } + } else { + scope_cache = NewHash(); + Setattr(s_cache, scope_name, scope_cache); + Delete(scope_name); } #endif @@ -2025,8 +2036,8 @@ SwigType *Swig_symbol_template_deftype(const SwigType *type, Symtab *tscope) { } Delete(elements); #ifdef SWIG_TEMPLATE_DEFTYPE_CACHE - Setattr(deftype_cache, scopetype, result); - Delete(scopetype); + Setattr(scope_cache, type_name, result); + Delete(type_name); #endif return result; From 974d822b5f9890050df0b6002dfc81b7a9a82937 Mon Sep 17 00:00:00 2001 From: Frank Schlimbach Date: Wed, 11 Jan 2017 03:10:40 -0600 Subject: [PATCH 5/7] template_default_cache is a multi-module test --- Examples/test-suite/common.mk | 2 +- Examples/test-suite/template_default_cache.list | 2 ++ 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 Examples/test-suite/template_default_cache.list diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index d77c43751..adc3ff948 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -407,7 +407,6 @@ CPP_TEST_CASES += \ template_default_arg_overloaded \ template_default_arg_overloaded_extend \ template_default_arg_virtual_destructor \ - template_default_cache \ template_default_class_parms \ template_default_class_parms_typedef \ template_default_inherit \ @@ -671,6 +670,7 @@ MULTI_CPP_TEST_CASES += \ packageoption \ mod \ template_typedef_import \ + template_default_cache \ multi_import # Custom tests - tests with additional commandline options diff --git a/Examples/test-suite/template_default_cache.list b/Examples/test-suite/template_default_cache.list new file mode 100644 index 000000000..d810c2c87 --- /dev/null +++ b/Examples/test-suite/template_default_cache.list @@ -0,0 +1,2 @@ +amodel +template_default_cache From 6f54a00db7591e1f90964ad39e0929c6e2253691 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Mon, 16 Jan 2017 07:19:05 +0000 Subject: [PATCH 6/7] Fix template_default_cache testcase --- Examples/test-suite/amodel.i | 10 ---------- Examples/test-suite/common.mk | 2 +- Examples/test-suite/template_default_cache.i | 2 -- Examples/test-suite/template_default_cache.list | 2 -- 4 files changed, 1 insertion(+), 15 deletions(-) delete mode 100644 Examples/test-suite/amodel.i delete mode 100644 Examples/test-suite/template_default_cache.list diff --git a/Examples/test-suite/amodel.i b/Examples/test-suite/amodel.i deleted file mode 100644 index efb394328..000000000 --- a/Examples/test-suite/amodel.i +++ /dev/null @@ -1,10 +0,0 @@ -%module amodel; - -%inline %{ -namespace ns__a { - namespace iface1 { - class Model {}; - typedef d::d ModelPtr; - } -} - %} diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index adc3ff948..d77c43751 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -407,6 +407,7 @@ CPP_TEST_CASES += \ template_default_arg_overloaded \ template_default_arg_overloaded_extend \ template_default_arg_virtual_destructor \ + template_default_cache \ template_default_class_parms \ template_default_class_parms_typedef \ template_default_inherit \ @@ -670,7 +671,6 @@ MULTI_CPP_TEST_CASES += \ packageoption \ mod \ template_typedef_import \ - template_default_cache \ multi_import # Custom tests - tests with additional commandline options diff --git a/Examples/test-suite/template_default_cache.i b/Examples/test-suite/template_default_cache.i index fd38a441d..b7ef9e869 100644 --- a/Examples/test-suite/template_default_cache.i +++ b/Examples/test-suite/template_default_cache.i @@ -1,7 +1,5 @@ %module template_default_cache; -%import "amodel.i" - %inline %{ namespace d { template< typename T > class d {}; diff --git a/Examples/test-suite/template_default_cache.list b/Examples/test-suite/template_default_cache.list deleted file mode 100644 index d810c2c87..000000000 --- a/Examples/test-suite/template_default_cache.list +++ /dev/null @@ -1,2 +0,0 @@ -amodel -template_default_cache From 6db71c690a8ef6032eab89298fd98c0550bf285f Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Mon, 16 Jan 2017 07:46:03 +0000 Subject: [PATCH 7/7] Add template_default_cache runtime tests Also correct illegal C++ namespace names --- .../java/template_default_cache_runme.java | 18 ++++++++++++++++++ .../python/template_default_cache_runme.py | 9 +++++++++ Examples/test-suite/template_default_cache.i | 14 +++++++------- 3 files changed, 34 insertions(+), 7 deletions(-) create mode 100644 Examples/test-suite/java/template_default_cache_runme.java create mode 100644 Examples/test-suite/python/template_default_cache_runme.py diff --git a/Examples/test-suite/java/template_default_cache_runme.java b/Examples/test-suite/java/template_default_cache_runme.java new file mode 100644 index 000000000..90b584261 --- /dev/null +++ b/Examples/test-suite/java/template_default_cache_runme.java @@ -0,0 +1,18 @@ +import template_default_cache.*; + +public class template_default_cache_runme { + + static { + try { + System.loadLibrary("template_default_cache"); + } 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[]) { + AModelPtr ap = template_default_cache.get_mp_a(); + BModelPtr bp = template_default_cache.get_mp_b(); + } +} diff --git a/Examples/test-suite/python/template_default_cache_runme.py b/Examples/test-suite/python/template_default_cache_runme.py new file mode 100644 index 000000000..ffe155acf --- /dev/null +++ b/Examples/test-suite/python/template_default_cache_runme.py @@ -0,0 +1,9 @@ +import template_default_cache + +ap = template_default_cache.get_mp_a(); +bp = template_default_cache.get_mp_b(); + +if not isinstance(ap, template_default_cache.AModelPtr): + raise RuntimeError("get_mp_a fail") +if not isinstance(bp, template_default_cache.BModelPtr): + raise RuntimeError("get_mp_b fail") diff --git a/Examples/test-suite/template_default_cache.i b/Examples/test-suite/template_default_cache.i index b7ef9e869..70bccd595 100644 --- a/Examples/test-suite/template_default_cache.i +++ b/Examples/test-suite/template_default_cache.i @@ -6,10 +6,10 @@ } %} -%ignore ns__a::iface1::Model; +%ignore ns_a::iface1::Model; %inline %{ -namespace ns__a { +namespace ns_a { namespace iface1 { class Model {}; typedef d::d ModelPtr; @@ -19,17 +19,17 @@ namespace ns__a { %} %inline %{ -namespace ns__b { +namespace ns_b { namespace iface1 { class Model { public: - ns__a::ModelPtr foo() { return ns__a::ModelPtr(); }; + ns_a::ModelPtr foo() { return ns_a::ModelPtr(); }; }; typedef d::d ModelPtr; - ns__a::ModelPtr get_mp_a() { return ns__a::ModelPtr(); } + ns_a::ModelPtr get_mp_a() { return ns_a::ModelPtr(); } ModelPtr get_mp_b() { return ModelPtr(); } } } %} -%template(AModelPtr) d::d; -%template(BModelPtr) d::d; +%template(AModelPtr) d::d; +%template(BModelPtr) d::d;