SWIG 4 Template Directive Changes (#1741)

* Updated convert_swig to account for template directive changes in SWIG 4

* Fixed merge issues

* Merge cleanup

docs/developer_docs/DesConvertSwig.md

@@ -275,10 +275,13 @@ Add the <b>class_string</b> to the SWIG interface text.
 1. While there's class content text remaining to be processed,
 repeatedly search for data members that match : <b>template_name '<'</b> <i>template-params</i> <b>'>' name ;</b>
 For each match, create a SWIG %template directive to create an instantiation
-of the specific templated type used by the data member. Add the
-SWIG %template directive to the templated typedefs string that
-Otherwise append whatever wasn't matched in process contents to
-the SWIG interface text.
+of the specific templated type used by the data member. Due to changes in SWIG 4,
+template directives must be specified before their respective data members. As such,
+the template directives are inserted immediately prior to the class definition. 
+SWIG does not resolve namespaces in these directives the same as C++ does 
+(it will only check local scope, not global). To account for this, 
+if the directive specifies a template outside the local namespace, convert_swig will
+escape the current namespace in the format : } template_directive namespace name {
 
 ## process_typedef_struct
 

libexec/trick/convert_swig

@@ -617,17 +617,15 @@ sub process_class($$$$$) {
                       \s*[\{\:]$
                      /sx or die "Internal error" ;
     $class_name = $1 ;
-
-    my $class_content;
-    $class_content .= $class_string ;
+    my $my_class_contents = $class_string ;
 
     if ( $class_string !~ /\{$/ ) {
         $$contents_ref =~ s/^(.*?\s*\{)//s ;
-        $class_content .= $1 ;
+        $my_class_contents .= $1 ;
     }
 
     # Add _swig_setattr_nondynamic_instance_variable function for raising AttributeError for improper class attribute assingment in input processor
-    $class_content .= "\n#if SWIG_VERSION > 0x040000\n\%pythoncode \%{\n    __setattr__ = _swig_setattr_nondynamic_instance_variable(object.__setattr__)\n\%}\n#endif\n" ;
+    $my_class_contents .= "\n#if SWIG_VERSION > 0x040000\n\%pythoncode \%{\n    __setattr__ = _swig_setattr_nondynamic_instance_variable(object.__setattr__)\n\%}\n#endif\n" ;
 
     ($extracted, $$contents_ref) = extract_bracketed( "{" . $$contents_ref , "{}") ;
 
@@ -639,20 +637,11 @@ sub process_class($$$$$) {
 
     #print "*** extracted = $extracted ***\n" ;
     #print "*** contents = $$contents_ref ***\n" ;
-    my $save_namespace_content;
-
-    if ( $curr_namespace ne "" ) {
-       my @split_namespaces = split "::", $curr_namespace;
-       my $sanitized_namespace = $split_namespaces[-1] ;
-       my @namespace_split = split /namespace\s*$sanitized_namespace/, $$new_contents_ref;
-       $save_namespace_content = 'namespace ' . $sanitized_namespace . $namespace_split[-1];
-       $$new_contents_ref = join('namespace ' . $sanitized_namespace, @namespace_split[0 .. $#namespace_split-1]);
-    }
 
     # SWIG doesn't like "const static".  Change it to "static const"
     $extracted =~ s/const\s+static/static const/g ;
 
-    my $isSwigExcludeBlock = 0;
+    my $isSwigExcludeBlock = 0 ;
 
     # templated variables need to be declared with the SWIG %template directive.
     # This loop looks for any templated variables and creates the %template lines.
@@ -662,15 +651,25 @@ sub process_class($$$$$) {
 
         if ( $non_var ne "" )  {
             #print "*** non_var = $non_var ***\n" ;
-            $class_content .= $non_var ;
+            $my_class_contents .= $non_var ;
             my $ifndefSwig = $non_var;
-            if ($isSwigExcludeBlock == 0) {
-               if ($ifndefSwig =~ /(?:ifndef\s*SWIG|if\s*!\s*defined\s*\(\s*SWIG\s*\))/ ) {
-                  $isSwigExcludeBlock = 1;
+            my $moreNonVar = 1 ;
+            # search for all instances of #ifndef SWIG, #if !defined(SWIG), and #endif prior to template variable
+            # update $isSwigExcludeBlock to the last instance*
+            # exit when no match is found
+            # *  this script does not track preprocessor scope, so any #endif will set $isSwigExcludeBlock to 0
+            #    in other words we don't support SWIGing nested preprocessor if statements, use at your peril
+            while ($moreNonVar == 1) {
+               if ($ifndefSwig =~ s/(#\s*ifndef\s*SWIG)|(#\s*if\s*!\s*defined\s*\(\s*SWIG\s*\))|(#\s*endif\s*)// ) {
+                  if($1 ne "" or $2 ne "") {
+                     $isSwigExcludeBlock = 1 ;
+                  }
+                  elsif($3 ne "") {
+                     $isSwigExcludeBlock = 0 ;
+                  }
                }
-            } else {
-               if ($ifndefSwig =~ /endif/ ) {
-                  $isSwigExcludeBlock = 1;
+               else {
+                  $moreNonVar = 0 ;
                }
             }
         }
@@ -681,6 +680,7 @@ sub process_class($$$$$) {
                 $template_var_def_str =~ /(.*?>)\s*([_A-Za-z]\w*).*?;/s ;
                 my ($template_full_type) = $1 ;
                 my ($var_name) = $2 ;
+                $my_class_contents .= $template_var_def_str ;
 
                 $template_full_type =~ /([_A-Za-z][:\w]*)\s*</ ;
                 my ($template_type) = $1 ;
@@ -703,28 +703,34 @@ sub process_class($$$$$) {
                         my $identifier = "${sanitized_namespace}${class_name}_${var_name}" ;
                         my $trick_swig_template = "TRICK_SWIG_TEMPLATE_$identifier" ;
 
-                        my $typedef;
-                        if ($isSwigExcludeBlock == 0) {
-                           $typedef = "#ifndef $trick_swig_template\n" ;
-                           $typedef   .= "#define $trick_swig_template\n";
-                           if ($qualified) {
-                              $typedef .= "\%template($identifier) $template_full_type;\n" ;
-                           } else {
-                              if ( $curr_namespace ne "" ) {
-                                 my $cppns = substr $curr_namespace, 0, -2;
-                                 $typedef .= "namespace $cppns {\n";
-                                 $typedef .= "\%template($identifier) $template_full_type;\n}\n" ;
-                              } else {
-                                 $typedef .= "\%template($identifier) $template_full_type;\n" ;
-                              }
-                           }
-                           $typedef   .= "#endif\n\n" ;
-                           $$new_contents_ref .= $typedef ;
+                        # Insert template directive immediately before intsance
+                        # This is required as of SWIG 4
+                        my $typedef = "\n#ifndef $trick_swig_template\n" ;
+                        $typedef   .= "#define $trick_swig_template\n" ;
+                        $typedef   .= "\%template($identifier) $template_full_type;\n" ;
+                        $typedef   .= "#endif\n" ;
+
+                        # SWIG namespace resolution for template directives starts at the local space
+                        # Therefore, if the type is qualified, assume it's fully qualified and put the
+                        # %template directive in the global namespace by escaping the current namespace
+			if ($curr_namespace ne "") {
+                            my $in_same_namespace = 1 ;
+                            if ($template_full_type =~ /^\w*(::)\w+</) {
+                                $in_same_namespace = 0 ;
+                            }
+                            if ($in_same_namespace eq 0) {
+                                $curr_namespace =~ /(.*)::/ ;
+                                $typedef = "\n}" . $typedef . "namespace " . $1 . " {" ; 
+                            }
                         }
+
+                        if ($isSwigExcludeBlock == 0) {
+                            $template_typedefs .= $typedef ;
+                        }
+
                         $processed_templates{$template_type_no_sp} = 1 ;
                     }
                 }
-                $class_content .= $template_var_def_str ;
             }
         }
     }
@@ -732,18 +738,18 @@ sub process_class($$$$$) {
 
     push @$class_names_ref , "$curr_namespace$class_name" ;
 
+    # write out the templated variable declaration lines found in this class.
+    $$new_contents_ref .= $template_typedefs."\n" ;
+
+    $$new_contents_ref .= $my_class_contents ;
     # write the class contents and semicolon to ensure any template declarations below are after the semicolon.
-    $class_content .= $extracted . ";\n" ;
+    $$new_contents_ref .= $extracted . ";\n" ;
 
     my $c_ = "$curr_namespace$class_name" ;
     $c_ =~ s/\:/_/g ;
     # Add a #define line that signals that this class has been processed by swig. Classes excluded in #if 0 blocks will
     # not have this #define defined.
-    $class_content .= "#define TRICK_SWIG_DEFINED_$c_" ;
-    if ( $save_namespace_content ne "" ) {
-       $$new_contents_ref .= $save_namespace_content;
-    }
-    $$new_contents_ref .= $class_content;
+    $$new_contents_ref .= "#define TRICK_SWIG_DEFINED_$c_" ;
 }
 
 ## ================================================================================

test/SIM_test_ip/RUN_test/unit_test.py

@@ -2995,6 +2995,24 @@ def main():
     TRICK_EXPECT_TRUE( test_so.test_true(), test_suite , "boolean function return" )
     TRICK_EXPECT_FALSE( test_so.test_false(), test_suite , "boolean function return" )
 
+######################################################################################################################
+
+    test_suite = "SWIG Templates"
+
+    test_so.obj.class_no_ns.tnns.x = 1
+    test_so.obj.class_no_ns.tns.y  = 2
+    test_so.obj.class_ns.tnns.x = 3
+    test_so.obj.class_ns.tns.y  = 4
+    test_so.obj.foo1.bar.z  = 5
+    test_so.obj.foo2.bar.z  = 6
+
+    TRICK_EXPECT_EQ( test_so.obj.class_no_ns.tnns.x, 1, test_suite , "template member access" )
+    TRICK_EXPECT_EQ( test_so.obj.class_no_ns.tns.y, 2, test_suite , "template member access" )
+    TRICK_EXPECT_EQ( test_so.obj.class_ns.tnns.x, 3, test_suite , "template member access" )
+    TRICK_EXPECT_EQ( test_so.obj.class_ns.tns.y, 4, test_suite , "template member access" )
+    TRICK_EXPECT_EQ( test_so.obj.foo1.bar.z, 5, test_suite , "template member access" )
+    TRICK_EXPECT_EQ( test_so.obj.foo2.bar.z, 6, test_suite , "template member access" )
+
 ######################################################################################################################
 
 if __name__ == "__main__":

test/SIM_test_ip/models/test_ip/include/ClassOfEverything.hh

@@ -28,6 +28,8 @@ LIBRARY DEPENDENCY:
 #include "test_ip/include/NoICG.hh"
 #include "exclude_me/include/exclude_me.hh"
 #include "test_ip/include/Namespace_tests.hh"
+#include "test_ip/include/TemplateTest.hh"
+#include "test_ip/include/IfndefSwigTest.hh"
 
 /** @class Ball
     @brief ball in C++
@@ -441,6 +443,12 @@ class ClassOfEverything {
    std::map < std::string , int > msi ;
    std::list < std::string  > ls ;
 
+   ClassNoNS class_no_ns ;
+   NS2::ClassNS class_ns ;
+
+   a::Foo foo1;
+   b::Foo2 foo2;
+
    private:
        ClassOfEverything (const ClassOfEverything &);
        ClassOfEverything & operator= (const ClassOfEverything &);

test/SIM_test_ip/models/test_ip/include/FooB.hh

@@ -0,0 +1,35 @@
+/********************************* TRICK HEADER *******************************
+PURPOSE:
+    ()
+LIBRARY DEPENDENCY:
+    ()
+PROGRAMMERS:
+    (((Your Name) (Company Name) (Date) (Trick tutorial)))
+*******************************************************************************/
+
+#ifndef FOOB_HH_
+#define FOOB_HH_
+
+namespace FooNamespace
+{
+
+// RestartableStateIntegrator
+template<int T> 
+class FooA
+{
+public:
+
+   int A = T;
+};
+
+// RestartableFirstOrderODEIntegrator
+template<int T>
+class FooB : public FooA<3>
+{
+public:
+   int B = T;
+};
+
+}
+
+#endif

test/SIM_test_ip/models/test_ip/include/FooD.hh

@@ -0,0 +1,36 @@
+/********************************* TRICK HEADER *******************************
+PURPOSE:
+    ()
+LIBRARY DEPENDENCY:
+    ()
+PROGRAMMERS:
+    (((Your Name) (Company Name) (Date) (Trick tutorial)))
+*******************************************************************************/
+
+#ifndef FOOD_HH_
+#define FOOD_HH_
+
+#include "FooB.hh"
+
+namespace FooNamespace
+{
+
+// RestartableSecondOrderODEIntegrator
+class FooC : public FooA<1>
+{
+public:
+   int C;
+};
+
+// RestartableSimpleSecondOrderODEIntegrator
+template<int T>
+class FooD : public FooC
+{
+public:
+   int D = T;
+};
+
+
+}
+
+#endif

test/SIM_test_ip/models/test_ip/include/IfndefSwigTest.hh

@@ -0,0 +1,62 @@
+/********************************* TRICK HEADER *******************************
+PURPOSE:
+    ()
+LIBRARY DEPENDENCY:
+    ()
+PROGRAMMERS:
+    (((Your Name) (Company Name) (Date) (Trick tutorial)))
+*******************************************************************************/
+
+#ifndef FOO_HH_
+#define FOO_HH_
+
+#include "FooB.hh"
+#include "FooD.hh"
+
+namespace FooNamespace
+{
+
+// RestartableScalarFirstOrderODEIntegrator
+class FooContB {
+public:
+  FooContB() : d(12.0) {}
+
+   // RestartableFirstOrderODEIntegrator
+   FooB<2> fooB;
+
+  FooB<2> fooB2;
+
+  double d;
+};
+
+
+// RestartableT3SecondOrderODEIntegrator
+class FooContD {
+public:
+  FooContD() : d(12.0) {}
+
+  // RestartableSimpleSecondOrderODEIntegrator
+#ifndef TESTING_SWIG  
+# ifndef SWIG 
+// THIS SHOULD PREVENT SWIG FROM MAKING ANY TEMPLATE REFERENCES TO EXCLUDED FooD TYPE
+  FooD<1> fooD;
+
+  FooD<2> fooD2;
+#endif
+#endif
+
+  int spacer;
+
+#if ! defined ( SWIG ) 
+// THIS SHOULD PREVENT SWIG FROM MAKING ANY TEMPLATE REFERENCES TO EXCLUDED FooD TYPE
+  FooD<3> fooD3;
+
+  FooD<4> fooD4;
+#endif
+
+  double d;
+};
+
+}
+
+#endif

test/SIM_test_ip/models/test_ip/include/TemplateTest.hh

@@ -11,10 +11,10 @@ PURPOSE:
 #define TEMPLATETEST_HH
 
 template <class A, class B>
-class TTT {
+class TTT_test {
     public:
 
-        TTT() {
+        TTT_test() {
             aa = 0 ;
             bb = 0 ;
             cc = NULL ;
@@ -22,11 +22,11 @@ class TTT {
         } ;
         A aa ;
         B bb ;
-        TTT<A,B> * ttt ;
+        TTT_test<A,B> * ttt ;
 
-        typedef TTT<A,B> C ;
+        typedef TTT_test<A,B> C ;
         C * cc ;
-        typedef TTT<B,A> D ;
+        typedef TTT_test<B,A> D ;
         D * dd ;
 } ;
 
@@ -36,7 +36,7 @@ class TemplateTest {
   friend void init_attrTemplateTest() ;
 
   public:
-   TTT< int , double > TTT_var ;
+   TTT_test< int , double > TTT_var ;
 
 };
 
@@ -44,5 +44,55 @@ class TemplateTest {
 %struct_str(TemplateTest)
 #endif
 
+//Verify we can build templates/intsantiations defined in different combinations of namespaces
+template <class T>
+struct TemplateNoNS {T x;}; 
+
+namespace NS1 {
+	template <class T>
+	struct TemplateNS {T y;}; 
+}
+
+class ClassNoNS {
+  public:
+    TemplateNoNS<int> tnns;
+    NS1::TemplateNS<int> tns;
+};
+
+namespace NS2 {
+	class ClassNS {
+	  public:
+	    TemplateNoNS<int> tnns;
+	    NS1::TemplateNS<int> tns;
+	};
+}
+
+//Verify we can build templates/intsantiations defined in the same namespace
+namespace a { 
+
+	template <class T>
+	struct Bar {T z;}; 
+
+	class Foo {
+	  public:
+	    Bar<int> bar;
+	};
+
+}
+
+//Verify we can build templates/intsantiations defined in different namespaces
+namespace b { 
+
+	class Foo2 {
+	  public:
+	    a::Bar<int> bar;
+	};
+
+}
+
+//Verify we can build with templated functions (isn't actually SWIG-ified, but should be ignored)
+template <typename T> void templated_function() {}
+
+
 #endif /* _BALL_HH_ */