From ebed6508e4cc3cee2a782fd6e15d6017c8cdcb25 Mon Sep 17 00:00:00 2001 From: William S Fulton Date: Wed, 11 Nov 2009 00:30:34 +0000 Subject: [PATCH] Nested class improvements - Fixed inconsistency in handling C++ nested classes - sometimes they were treated as forward declarations, other times as if C nested struct was parsed. Added the nestedworkaround feature for C++ nested class handling. Document improved nested class handling. Numerous C and C++ nested struct/class/union test cases added. git-svn-id: https://swig.svn.sourceforge.net/svnroot/swig/trunk@11734 626c5289-ae23-0410-ae9c-e8d60b6d4f22 --- CHANGES.current | 24 +++- Doc/Manual/Introduction.html | 2 +- Doc/Manual/SWIG.html | 14 ++- Doc/Manual/SWIGPlus.html | 107 +++++++++++++----- Examples/test-suite/common.mk | 1 + Examples/test-suite/nested.i | 21 ++++ Examples/test-suite/nested_class.i | 60 ++++++++-- Examples/test-suite/nested_workaround.i | 38 +++++++ .../python/nested_workaround_runme.py | 13 +++ Lib/swig.swg | 5 + Source/CParse/parser.y | 94 ++++++++------- Source/Swig/symbol.c | 3 +- 12 files changed, 300 insertions(+), 82 deletions(-) create mode 100644 Examples/test-suite/nested_workaround.i create mode 100644 Examples/test-suite/python/nested_workaround_runme.py diff --git a/CHANGES.current b/CHANGES.current index bfcb65514..e03d6773a 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -1,12 +1,30 @@ Version 1.3.41 (in progress) ============================ +2009-11-11: wsfulton + Added the nestedworkaround feature as a way to use the full functionality of a nested class + (C++ mode only). It removes the nested class from SWIG's type information so it is as if SWIG + had never parsed the nested class. The documented nested class workarounds using a global + fake class stopped working when SWIG treated the nested class as an opaque pointer, and + this feature reverts this behaviour. The documentation has been updated with details of how + to use and implement it, see the "Nested classes" section in SWIGPlus.html. + +2009-11-11: wsfulton + There were a number of C++ cases where nested classes/structs/unions were being handled + as if C code was being parsed which would oftentimes lead to uncompileable code as an + attempt was made to wrap the nested structs like it is documented for C code. Now all + nested structs/classes/unions are ignored in C++ mode, as was always documented. However, + there is an improvement as usage of nested structs/classes/unions is now always treated + as an opaque type by default, resulting in generated code that should always compile. + + *** POTENTIAL INCOMPATIBILITY *** + 2009-11-09: drjoe Fix R for -fcompact and add std_map.i 2009-11-08: wsfulton - Fix inconsistency for inner structs/unions/classes. Uncompileable code was being - generated when inner struct and union declarations were used as types within - the inner struct. The inner struct/union is now treated as a forward declaration making the + Fix inconsistency for nested structs/unions/classes. Uncompileable code was being + generated when inner struct and union declarations were used as types within the + inner struct. The inner struct/union is now treated as a forward declaration making the behaviour the same as an inner class. (C++ code), eg: struct Outer { diff --git a/Doc/Manual/Introduction.html b/Doc/Manual/Introduction.html index 099454cf0..df8d03fdf 100644 --- a/Doc/Manual/Introduction.html +++ b/Doc/Manual/Introduction.html @@ -334,7 +334,7 @@ major features include:

Currently, the only major C++ feature not supported is nested classes--a limitation -that will be removed in a future release. +that should be removed in a future release, but has some workarounds for the moment.

diff --git a/Doc/Manual/SWIG.html b/Doc/Manual/SWIG.html index 8705fa182..dd1b22095 100644 --- a/Doc/Manual/SWIG.html +++ b/Doc/Manual/SWIG.html @@ -334,6 +334,7 @@ currently supported: For example, SWIG does not support declarations such as the following (even though this is legal C): +

 /* Non-conventional placement of storage specifier (extern) */
@@ -347,6 +348,7 @@ void bar(Spam (Grok)(Doh));
+

In practice, few (if any) C programmers actually write code like @@ -360,6 +362,7 @@ is not recommended. Even though SWIG can parse C++ class declarations, it ignores declarations that are decoupled from their original class definition (the declarations are parsed, but a lot of warning messages may be generated). For example: +

@@ -369,11 +372,12 @@ int foo::bar(int) {
 }
+

  • Certain advanced features of C++ such as nested classes -are not yet supported. Please see the section on using SWIG -with C++ for more information. +are not yet fully supported. Please see the C++ section +for more information.

    @@ -2633,6 +2637,12 @@ If you have a lot nested structure declarations, it is advisable to double-check them after running SWIG. Although, there is a good chance that they will work, you may have to modify the interface file in certain cases. +

    + +

    +Finally, note that nesting is handled differently in C++ mode, +see Nested classes. +

    5.5.8 Other things to note about structure wrapping

    diff --git a/Doc/Manual/SWIGPlus.html b/Doc/Manual/SWIGPlus.html index f318af6a3..936fd3fb1 100644 --- a/Doc/Manual/SWIGPlus.html +++ b/Doc/Manual/SWIGPlus.html @@ -4695,37 +4695,49 @@ public:

    -There is limited support for nested structs and unions when wrapping C code, see Nested structures for further details. -However, there is no nested class/struct/union support when wrapping C++ code (using the -c++ commandline option). -This may be added at a future date, however, until then some of the following workarounds can be applied. +There is some support for nested structs and unions when wrapping C code, +see Nested structures for further details. +The added complexity of C++ compared to C means this approach does not work well for +C++ code (when using the -c++ command line option). +For C++, a nested class is treated much like an opaque pointer, so anything useful within the nested class, such as its +methods and variables, are not accessible from the target language. +True nested class support may be added to SWIG in the future, however, +until then some of the following workarounds can be applied to improve the situation.

    -It might be possible to use partial class information. Since -SWIG does not need the entire class specification to work, conditional -compilation can be used to comment out the problematic nested class definition, you might do this: +It might be possible to use partial class information as often you can accept that the nested class is not needed, +especially if it is not actually used in any methods you need from the target language. +Imagine you are wrapping the following Outer class which contains a nested class Inner. +The easiest thing to do is turn a blind eye to the warning that SWIG generates, or simply suppress it: 

    -class Foo {
+#pragma SWIG nowarn=SWIGWARN_PARSE_NESTED_CLASS
+class Outer {
 public:
-#ifndef SWIG
-   class Bar {
-   public:
-     ...
-   };
-#endif
-   Foo();
-  ~Foo();
-   ...
+  class Inner {
+    public:
+      ...
+  };
+  Inner getInner();
+  void useInner(const Inner& inner);
+  ...
 };

    -The next workaround assumes you cannot modify the source code as was done above and it provides a solution for methods that use nested class types. -Imagine we are wrapping the Outer class which contains a nested class Inner: +Note that if Inner can be used as an opaque type, the default wrapping approach suffices. +For example, if the nested class does not need to be created from the target language, but can be obtained via a method +call, such as the getInner() method above, the returned value can then be passed around, such as passed into the +useInner() method. +

    + +

    +With some more effort the above situation can be improved somewhat and a nested class can be constructed and used +from the target language much like any other non-nested class. Assuming we have the Outer class in a header file:

    @@ -4738,14 +4750,18 @@ public: int var; Inner(int v = 0) : var(v) {} }; - void method(Inner inner); + Inner getInner(); + void useInner(const Inner& inner); };

    -The following interface file works around SWIG nested class limitations by redefining the nested class as a global class. -A typedef for the compiler is also required in order for the generated wrappers to compile. +The following interface file works around the nested class limitations by redefining the nested class as a global class. +A typedef for the compiler and the nestedworkaround +feature flag is also required in +order for the generated wrappers to compile. This flag simply removes all the type information from SWIG, so SWIG treats +the nested class as if it had not been parsed at all.

    @@ -4753,9 +4769,6 @@ A typedef for the compiler is also required in order for the generated wrappers // File : example.i %module example -// Suppress SWIG warning -#pragma SWIG nowarn=SWIGWARN_PARSE_NESTED_CLASS - // Redefine nested class in global scope in order for SWIG to generate // a proxy class. Only SWIG parses this definition. class Inner { @@ -4764,24 +4777,62 @@ class Inner { Inner(int v = 0) : var(v) {} }; +%nestedworkaround Outer::Inner; + %{ #include "outer.h" %} %include "outer.h" +// We've fooled SWIG into thinking that Inner is a global class, so now we need +// to trick the C++ compiler into understanding this apparent global type. %{ -// SWIG thinks that Inner is a global class, so we need to trick the C++ -// compiler into understanding this so called global type. typedef Outer::Inner Inner; %} -

    -The downside to this approach is having to maintain two definitions of Inner, the real one and the one in the interface file that SWIG parses. +The downside to this approach is a more complex interface file and having to maintain two definitions of Inner, +the real one and the one in the interface file that SWIG parses. +However, the upside is that all the methods/variables in the nested class are available from the target language +as a proxy class is generated instead of treating the nested class as an opaque type. +The proxy class can be constructed from the target language and passed into any methods accepting the nested class. +Also note that the original header file is parsed unmodified.

    +

    +Finally, conditional compilation can be used as a workaround to comment out nested class definitions in the actual headers, +assuming you are able to modify them. +

    + +
    +
    +// File outer.h
+class Outer {
+public:
+#ifndef SWIG
+  class Inner {
+    public:
+      ...
+  };
+#endif
+  ...
+};
+
    +
    + +

    +This workaround used to be common when SWIG could not deal with nested classes particulary well. +This should just be a last resort for unusual corner cases now as SWIG can parse nested classes and even handle nested template classes fairly well. +

    + +

    +Compatibility Note: SWIG-1.3.40 and earlier versions did not have the nestedworkaround feature +and the generated code resulting from parsing nested classes did not always compile. +

    + +

    6.27 A brief rant about const-correctness

    diff --git a/Examples/test-suite/common.mk b/Examples/test-suite/common.mk index cf9faaad9..93c1cc909 100644 --- a/Examples/test-suite/common.mk +++ b/Examples/test-suite/common.mk @@ -244,6 +244,7 @@ CPP_TEST_CASES += \ naturalvar \ nested_class \ nested_comment \ + nested_workaround \ newobject1 \ null_pointer \ operator_overload \ diff --git a/Examples/test-suite/nested.i b/Examples/test-suite/nested.i index 0b93be46a..004cb4814 100644 --- a/Examples/test-suite/nested.i +++ b/Examples/test-suite/nested.i @@ -12,6 +12,27 @@ struct TestStruct { int a; }; +struct OuterStructNamed { + struct InnerStructNamed { + double dd; + } inner_struct_named; + union InnerUnionNamed { + double ee; + int ff; + } inner_union_named; +}; + +struct OuterStructUnnamed { + struct { + double xx; + } inner_struct_unnamed; + union { + double yy; + int zz; + } inner_union_unnamed; +}; + + typedef struct OuterStruct { union { diff --git a/Examples/test-suite/nested_class.i b/Examples/test-suite/nested_class.i index ce45b2981..3b964f756 100644 --- a/Examples/test-suite/nested_class.i +++ b/Examples/test-suite/nested_class.i @@ -4,52 +4,94 @@ %inline %{ struct Outer { + typedef int Integer; + /////////////////////////////////////////// struct InnerStruct1 { - int x; + Integer x; }; class InnerClass1 { public: - int x; + Integer x; }; union InnerUnion1 { - int x; + Integer x; double y; }; + /////////////////////////////////////////// class { public: - int a; + Integer a; }; struct { - int b; + Integer b; }; union { - int c; + Integer c; double d; }; + /////////////////////////////////////////// class InnerClass2 { public: - int x; + Integer x; } InnerClass2Name; struct InnerStruct2 { - int x; + Integer x; } InnerStruct2Name; union InnerUnion2 { - int x; + Integer x; double y; } InnerUnion2Name; + /////////////////////////////////////////// + class { + public: + Integer x; + } InnerClass3Name; + + struct { + Integer x; + } InnerStruct3Name; + + union { + Integer x; + double y; + } InnerUnion3Name; + + /////////////////////////////////////////// + typedef class { + public: + Integer x; + } InnerClass4; + + typedef struct { + Integer x; + } InnerStruct4; + + typedef union { + Integer x; + double y; + } InnerUnion4; + // bug #909387 - inner declared types are treated as forward declarations InnerStruct1* getInnerStruct1() { return 0; } InnerClass1* getInnerClass1() { return 0; } InnerUnion1* getInnerUnion1() { return 0; } + + InnerStruct2* getInnerStruct2() { return 0; } + InnerClass2* getInnerClass2() { return 0; } + InnerUnion2* getInnerUnion2() { return 0; } + + InnerStruct4* getInnerStruct4() { return 0; } + InnerClass4* getInnerClass4() { return 0; } + InnerUnion4* getInnerUnion4() { return 0; } }; %} diff --git a/Examples/test-suite/nested_workaround.i b/Examples/test-suite/nested_workaround.i new file mode 100644 index 000000000..9727dacee --- /dev/null +++ b/Examples/test-suite/nested_workaround.i @@ -0,0 +1,38 @@ +%module nested_workaround +// Similar to "Nested classes" documentation example. + +class Inner { + int val; + public: + Inner(int v = 0) : val(v) {} + void setValue(int v) { val = v; } + int getValue() const { return val; } +}; +%nestedworkaround Outer::Inner; + +%inline %{ +class Outer { +public: + class Inner { + int val; + public: + Inner(int v = 0) : val(v) {} + void setValue(int v) { val = v; } + int getValue() const { return val; } + }; + Inner createInner(int v) const { return Inner(v); } + int getInnerValue(const Inner& i) const { return i.getValue(); } + Inner doubleInnerValue(Inner inner) { + inner.setValue(inner.getValue() * 2); + return inner; + } +}; +%} + +// We've fooled SWIG into thinking that Inner is a global class, so now we need +// to trick the C++ compiler into understanding this apparent global type. +%{ +typedef Outer::Inner Inner; +%} + + diff --git a/Examples/test-suite/python/nested_workaround_runme.py b/Examples/test-suite/python/nested_workaround_runme.py new file mode 100644 index 000000000..a8a75d370 --- /dev/null +++ b/Examples/test-suite/python/nested_workaround_runme.py @@ -0,0 +1,13 @@ +from nested_workaround import * + +inner = Inner(5) +outer = Outer() +newInner = outer.doubleInnerValue(inner) +if newInner.getValue() != 10: + raise RuntimeError + +outer = Outer() +inner = outer.createInner(3) +newInner = outer.doubleInnerValue(inner) +if outer.getInnerValue(newInner) != 6: + raise RuntimeError diff --git a/Lib/swig.swg b/Lib/swig.swg index c0ecad9dd..af7fa6a30 100644 --- a/Lib/swig.swg +++ b/Lib/swig.swg @@ -136,6 +136,11 @@ #define %nocallback %feature("callback","0") #define %clearcallback %feature("callback","") +/* the %nestedworkaround directive */ +#define %nestedworkaround %feature("nestedworkaround") +#define %nonestedworkaround %feature("nestedworkaround","0") +#define %clearnestedworkaround %feature("nestedworkaround","") + /* the %fastdispatch directive */ #define %fastdispatch %feature("fastdispatch") #define %nofastdispatch %feature("fastdispatch","0") diff --git a/Source/CParse/parser.y b/Source/CParse/parser.y index 9c68cc647..295f0a60c 100644 --- a/Source/CParse/parser.y +++ b/Source/CParse/parser.y @@ -4374,18 +4374,33 @@ cpp_protection_decl : PUBLIC COLON { by SWIG or this whole thing is going to puke. ---------------------------------------------------------------------- */ -/* A struct sname { } id; declaration */ +/* struct sname { } id; or struct sname { }; declaration */ cpp_nested : storage_class cpptype ID LBRACE { cparse_start_line = cparse_line; skip_balanced('{','}'); } nested_decl SEMI { $$ = 0; if (cplus_mode == CPLUS_PUBLIC) { - if ($6.id && strcmp($2, "class") != 0) { + if (cparse_cplusplus) { + /* Treat the nested class/struct/union as a forward declaration until a proper nested class solution is implemented */ + $$ = new_node("classforward"); + Setfile($$,cparse_file); + Setline($$,cparse_line); + Setattr($$,"kind",$2); + Setattr($$,"name",$3); + Setattr($$,"sym:weak", "1"); + add_symbols($$); + + if (GetFlag($$, "feature:nestedworkaround")) { + Swig_symbol_remove($$); + $$ = 0; + } else { + Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line, "Nested %s not currently supported (%s ignored).\n", $2, $3); + } + } else if ($6.id) { + /* Generate some code for a new struct */ Nested *n = (Nested *) malloc(sizeof(Nested)); n->code = NewStringEmpty(); - Printv(n->code, "typedef ", $2, " ", - Char(scanner_ccode), " $classname_", $6.id, ";\n", NIL); - + Printv(n->code, "typedef ", $2, " ", Char(scanner_ccode), " $classname_", $6.id, ";\n", NIL); n->name = Swig_copy_string($6.id); n->line = cparse_start_line; n->type = NewStringEmpty(); @@ -4394,50 +4409,53 @@ cpp_nested : storage_class cpptype ID LBRACE { cparse_start_line = cparse_line SwigType_push(n->type, $6.type); n->next = 0; add_nested(n); - } else { - Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line, "Nested %s not currently supported (%s ignored).\n", $2, $3); - - /* For now, just treat the nested class/struct/union as a forward - * declaration (SF bug #909387). */ - $$ = new_node("classforward"); - Setfile($$,cparse_file); - Setline($$,cparse_line); - Setattr($$,"kind",$2); - Setattr($$,"name",$3); - Setattr($$,"sym:weak", "1"); - add_symbols($$); } } } -/* A struct { } id; declaration */ + +/* struct { } id; or struct { }; declaration */ + | storage_class cpptype LBRACE { cparse_start_line = cparse_line; skip_balanced('{','}'); } nested_decl SEMI { $$ = 0; if (cplus_mode == CPLUS_PUBLIC) { - if (strcmp($2,"class") == 0) { - if ($5.id) - Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line,"Nested class not currently supported (%s ignored)\n", $5.id); - else - Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line,"Nested class not currently supported (ignored)\n"); - } else if ($5.id) { - /* Generate some code for a new class */ - Nested *n = (Nested *) malloc(sizeof(Nested)); - n->code = NewStringEmpty(); - Printv(n->code, "typedef ", $2, " " , - Char(scanner_ccode), " $classname_", $5.id, ";\n",NIL); - n->name = Swig_copy_string($5.id); - n->line = cparse_start_line; - n->type = NewStringEmpty(); - n->kind = $2; - n->unnamed = 1; - SwigType_push(n->type,$5.type); - n->next = 0; - add_nested(n); + if ($5.id) { + if (cparse_cplusplus) { + /* Treat the nested class/struct/union as a forward declaration until a proper nested class solution is implemented */ + $$ = new_node("classforward"); + Setfile($$,cparse_file); + Setline($$,cparse_line); + Setattr($$,"kind",$2); + Setattr($$,"name",$5.id); + Setattr($$,"sym:weak", "1"); + add_symbols($$); + + if (GetFlag($$, "feature:nestedworkaround")) { + Swig_symbol_remove($$); + $$ = 0; + } else { + Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line,"Nested %s not currently supported (%s ignored)\n", $2, $5.id); + } + } else { + /* Generate some code for a new struct */ + Nested *n = (Nested *) malloc(sizeof(Nested)); + n->code = NewStringEmpty(); + Printv(n->code, "typedef ", $2, " " , Char(scanner_ccode), " $classname_", $5.id, ";\n",NIL); + n->name = Swig_copy_string($5.id); + n->line = cparse_start_line; + n->type = NewStringEmpty(); + n->kind = $2; + n->unnamed = 1; + SwigType_push(n->type,$5.type); + n->next = 0; + add_nested(n); + } } else { Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line, "Nested %s not currently supported (ignored).\n", $2); } } } + /* A 'class name : base_list { };' declaration, always ignored */ /***** This fixes derived_nested.i, but it adds one shift/reduce. Anyway, @@ -4445,12 +4463,12 @@ cpp_nested : storage_class cpptype ID LBRACE { cparse_start_line = cparse_line *****/ | storage_class cpptype idcolon COLON base_list LBRACE { cparse_start_line = cparse_line; skip_balanced('{','}'); } SEMI { -Printf(stdout, "cpp_nested (c)\n"); $$ = 0; if (cplus_mode == CPLUS_PUBLIC) { Swig_warning(WARN_PARSE_NESTED_CLASS, cparse_file, cparse_line,"Nested %s not currently supported (%s ignored)\n", $2, $3); } } + /* This unfortunately introduces 4 shift/reduce conflicts, so instead the somewhat hacky nested_template is used for ignore nested template classes. */ /* | TEMPLATE LESSTHAN template_parms GREATERTHAN cpptype idcolon LBRACE { cparse_start_line = cparse_line; skip_balanced('{','}'); diff --git a/Source/Swig/symbol.c b/Source/Swig/symbol.c index ae8a274c6..b5ed37c0c 100644 --- a/Source/Swig/symbol.c +++ b/Source/Swig/symbol.c @@ -1331,7 +1331,8 @@ void Swig_symbol_remove(Node *n) { Setattr(symtab, symname, symnext); fixovername = symnext; /* fix as symbol to remove is at head of linked list */ } else { - Delattr(symtab, symname); + if (symname) + Delattr(symtab, symname); } } if (symnext) {