Skip to content

Commit aecf6a0

Browse files
authored
Improve SWIG %template directive scoping - take 2 (#1753)
* Correct convert_swig regex for qualified templates Closes #1751 * Improve SWIG %template directive scoping - take 2 Refs #768 * Add nested namespace tests Refs #768 * Replace ">>" with "> >" Probably time to drop support for CentOS 7, ya know. * Add SWIG template scoping documentation Refs #1753
1 parent e8508ea commit aecf6a0

File tree

5 files changed

+281
-127
lines changed

5 files changed

+281
-127
lines changed

docs/documentation/building_a_simulation/Model-Source-Code.md

Lines changed: 43 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -408,24 +408,51 @@ Trick may use model code with any type of inheritance. Some limitations are pres
408408

409409
### Namespaces
410410

411-
Currently one level of namespace is supported. Additional levels of namespaces are ignored. Similarly classes and enumerations embedded in other classes are ignored.
411+
ICG supports namespaces and nested scopes. Data recording and variable access via Trick View should work regardless of how many levels there are.
412412

413-
```C++
414-
namespace my_ns {
415-
// BB is processed
416-
class BB {
417-
public:
418-
std::string str;
419-
// Class CC is ignored.
420-
class CC {
421-
...
413+
Namespaces and nested scopes are similarly supported in Python contexts, such as the input file and variable server, with some caveats regarding templates.
414+
1. A template instantiation may be unqualified (have no use of the scope resolution operator `::`) only if its corresponding template is declared in the immediately-enclosing namespace.
415+
2. Otherwise, a template instantiation must be fully qualified, starting from the global namespace.
416+
3. Finally, instantiations of templates declared within the same class must be excluded from SWIG.
417+
418+
In the following examples, all template instantiations occur in `example::prime::Soup`. The immediately-enclosing namespace is `prime`, so only instantiations of templates declared directly in `prime` (only `Celery`) may be unqualified. All other template instantiations must be fully qualified, starting from the global namespace, even if the C++ name lookup process would find them with partial qualification.
419+
420+
```c++
421+
template <class T> class Potato {};
422+
423+
namespace example {
424+
425+
template <class T> class Onion {};
426+
427+
namespace peer {
428+
template <class T> class Raddish {};
429+
}
430+
431+
namespace prime {
432+
433+
namespace inner {
434+
template <class T> class Carrot {};
422435
}
423-
};
424-
// Everything enclosed in inner_ns is ignored.
425-
namespace inner_ns {
426-
...
427-
};
428-
};
436+
437+
template <class T> class Celery {};
438+
439+
class Soup {
440+
441+
public:
442+
template <class T> class Broth {};
443+
444+
::Potato<int> potato; // Rule 2
445+
example::Onion<int> onion; // Rule 2
446+
example::peer::Raddish<int> raddish; // Rule 2
447+
example::prime::inner::Carrot<int> carrot; // Rule 2
448+
Celery<int> celery; // Rule 1
449+
#ifndef SWIG
450+
Broth<int> broth; // Rule 3
451+
#endif
452+
};
453+
}
454+
455+
}
429456
```
430457
431458
### Function Overloading

libexec/trick/convert_swig

Lines changed: 60 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,6 @@ my %sim ;
5757
my %out_of_date ;
5858
my ($version, $thread, $year) ;
5959
my %processed_templates ;
60-
my $global_template_typedefs ;
6160

6261
my $typedef_def = qr/typedef\s+ # the word typedef
6362
(?:[_A-Za-z][\s\w]*\s*) # resolved type
@@ -336,7 +335,6 @@ sub process_file() {
336335
}
337336
print OUT "\n$new_contents" ;
338337
print OUT "$contents\n" ;
339-
print OUT $global_template_typedefs ;
340338

341339
# Add a trick_cast_as macro line for each class parsed in the file. These lines must appear at the bottom of the
342340
# file to ensure they are not in a namespace directive and they are after the #define statements they depend on.
@@ -592,7 +590,8 @@ sub process_class($$$$$) {
592590

593591
my $extracted ;
594592
my ($class_name) ;
595-
my $template_typedefs ;
593+
my @qualified_template_typedefs ;
594+
my @unqualified_template_typedefs ;
596595

597596
## Extract the class_name from the class_string
598597
$class_string =~ /^(?:class|struct)\s+ # keyword class or struct
@@ -675,40 +674,55 @@ sub process_class($$$$$) {
675674
my ($template_type_no_sp) = $template_full_type ;
676675
$template_type_no_sp =~ s/\s//g ;
677676

678-
# If the type is qualified, assume it's fully qualified and put the
679-
# %template directive in the global namespace.
680-
# See https://github.com/nasa/trick/issues/768
681-
my $qualified = $template_type_no_sp =~ /^\w+(::)\w+</ ;
682677
#print "*** template_type_no_sp = $template_type_no_sp ***\n" ;
683678
if ( ! exists $processed_templates{$template_type_no_sp} ) {
684679

685680
my $sanitized_namespace = $curr_namespace =~ s/:/_/gr ;
686681
my $identifier = "${sanitized_namespace}${class_name}_${var_name}" ;
687682
my $trick_swig_template = "TRICK_SWIG_TEMPLATE_$identifier" ;
688683

689-
# Insert template directive immediately before intsance
684+
# Insert template directive immediately before instance
690685
# This is required as of SWIG 4
691686
my $typedef = "\n#ifndef $trick_swig_template\n" ;
692687
$typedef .= "#define $trick_swig_template\n" ;
693688
$typedef .= "\%template($identifier) $template_full_type;\n" ;
694689
$typedef .= "#endif\n" ;
695690

696-
# SWIG namespace resolution for template directives starts at the local space
697-
# Therefore, if the type is qualified, assume it's fully qualified and put the
698-
# %template directive in the global namespace by escaping the current namespace
699-
if ($curr_namespace ne "") {
700-
my $in_same_namespace = 1 ;
701-
if ($template_full_type =~ /^\w*(::)\w+</) {
702-
$in_same_namespace = 0 ;
703-
}
704-
if ($in_same_namespace eq 0) {
705-
$curr_namespace =~ /(.*)::/ ;
706-
$typedef = "\n}" . $typedef . "namespace " . $1 . " {" ;
707-
}
708-
}
691+
# A SWIG %template directive must:
692+
# 1. Appear before each template instantiation
693+
# 2. Be in scope where the instantiation is declared
694+
# 3. Not be enclosed within a different namespace
695+
#
696+
# See https://www.swig.org/Doc4.2/SWIGPlus.html#SWIGPlus_template_scoping
697+
#
698+
# Generally, convert_swig is not smart enough to put all %template
699+
# directives in the right scope because:
700+
# 1. We do not keep track of what scope each type we encounter is
701+
# declared in.
702+
# 2. We cannot easily add %template directives to already-processed
703+
# scopes.
704+
#
705+
# As a compromise:
706+
# 1. For an unqualified template instantiation, the template declaration
707+
# is necessarily in the current or a containing scope. Hope it's in the
708+
# current namespace and put the %template directive there. This will
709+
# create invalid SWIG input code for templates declared in a containing
710+
# namespace and for member templates declared in this class.
711+
# 2. For a qualified template instantiation, the template declaration is
712+
# in a (possibly nested) scope contained by the current or a containing
713+
# scope. Assume the instantiation is fully qualified and put it in the
714+
# global namespace. This will create invalid SWIG code for partially-
715+
# qualified instantiations.
716+
#
717+
# See https://github.com/nasa/trick/issues/768
709718

710719
if ($isSwigExcludeBlock == 0) {
711-
$template_typedefs .= $typedef ;
720+
if ($template_full_type =~ /^\w*(::\w+)+</) {
721+
push @qualified_template_typedefs, $typedef ;
722+
}
723+
else {
724+
push @unqualified_template_typedefs, $typedef ;
725+
}
712726
}
713727

714728
$processed_templates{$template_type_no_sp} = 1 ;
@@ -721,12 +735,32 @@ sub process_class($$$$$) {
721735

722736
push @$class_names_ref , "$curr_namespace$class_name" ;
723737

724-
# write out the templated variable declaration lines found in this class.
725-
$$new_contents_ref .= $template_typedefs."\n" ;
738+
# Write out the %template directives for template instantiations found in
739+
# this class. Put the directives for unqualified instantiations in the
740+
# namespace containing this class, just before the class definition.
741+
foreach (@unqualified_template_typedefs) {
742+
$$new_contents_ref .= $_ ;
743+
}
744+
745+
# Assume qualified template instantiations are fully qualified and put
746+
# their %template directives in the global namespace.
747+
# 1. close all namespaces, returning to the global namespace
748+
# 2. add the %template directives
749+
# 3. open all namespaces, restoring the scope
750+
if (@qualified_template_typedefs) {
751+
my @namespaces = split(/::/, $curr_namespace) ;
752+
$$new_contents_ref .= "}" x @namespaces . "\n";
753+
foreach (@qualified_template_typedefs) {
754+
$$new_contents_ref .= $_ ;
755+
}
756+
$$new_contents_ref .= "\n";
757+
foreach (@namespaces) {
758+
$$new_contents_ref .= "namespace " . $_ . " { " ;
759+
}
760+
}
726761

727-
$$new_contents_ref .= $my_class_contents ;
728762
# write the class contents and semicolon to ensure any template declarations below are after the semicolon.
729-
$$new_contents_ref .= $extracted . ";\n" ;
763+
$$new_contents_ref .= "\n" . $my_class_contents . $extracted . ";\n" ;
730764

731765
my $c_ = "$curr_namespace$class_name" ;
732766
$c_ =~ s/\:/_/g ;
Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,2 @@
1-
TRICK_CONVERT_SWIG_FLAGS := -s
21
TRICK_CFLAGS += -Imodels
32
TRICK_CXXFLAGS += -Imodels

0 commit comments

Comments
 (0)