diff options
author | William S Fulton <wsf@fultondesigns.co.uk> | 2023-03-13 22:39:59 +0000 |
---|---|---|
committer | William S Fulton <wsf@fultondesigns.co.uk> | 2023-03-13 22:39:59 +0000 |
commit | 0c56f3557eeeee24ce438c78e35439dada5f5001 (patch) | |
tree | e70b9848df5cf55d99bbd150f82b82fedbd0b2e7 | |
parent | 7c043d4713a60bae693f9656d43222e842f9b61f (diff) | |
download | swig-0c56f3557eeeee24ce438c78e35439dada5f5001.tar.gz |
Improved error checking when defining classes and using %template.
1. When a template is instantiated via %template and uses the unary scope
operator ::, an error occurs if the instantiation is attempted within a
namespace that does not enclose the instantiated template.
For example, the following will now error as ::test::max is not enclosed within test1:
Error: '::test::max' resolves to 'test::max' and was incorrectly instantiated in
scope 'test1' instead of within scope 'test'.
namespace test1 {
%template(maxchar) ::test::max<char>;
}
2. SWIG previously failed to always detect a template did not exist when using
%template. In particular when instantiating a global template incorrectly within
namespace. The code below now correctly emits an error:
Error: Template 'test5::GlobalVector' undefined.
namespace test5 {
}
template<typename T> struct GlobalVector {};
%template(GVI) test5::GlobalVector<int>;
3. Also error out if an attempt is made to define a class using the unary scope
operator ::. The following is not legal C++ and now results in an error:
Error: Using the unary scope operator :: in class definition '::Space2::B' is invalid.
namespace Space2 {
struct B;
}
struct ::Space2::B {};
-rw-r--r-- | CHANGES.current | 35 | ||||
-rw-r--r-- | Examples/test-suite/errors/cpp_class_definition.i | 14 | ||||
-rw-r--r-- | Examples/test-suite/errors/cpp_class_definition.stderr | 2 | ||||
-rw-r--r-- | Examples/test-suite/errors/cpp_invalid_template.stderr | 1 | ||||
-rw-r--r-- | Examples/test-suite/errors/cpp_namespace_template_bad.i | 18 | ||||
-rw-r--r-- | Examples/test-suite/errors/cpp_namespace_template_bad.stderr | 20 | ||||
-rw-r--r-- | Examples/test-suite/errors/cpp_nested_template.stderr | 2 | ||||
-rw-r--r-- | Examples/test-suite/namespace_class.i | 6 | ||||
-rw-r--r-- | Examples/test-suite/template_partial_specialization.i | 21 | ||||
-rw-r--r-- | Source/CParse/parser.y | 100 |
10 files changed, 134 insertions, 85 deletions
diff --git a/CHANGES.current b/CHANGES.current index ea3e81022..29f5ec67b 100644 --- a/CHANGES.current +++ b/CHANGES.current @@ -7,6 +7,41 @@ the issue number to the end of the URL: https://github.com/swig/swig/issues/ Version 4.2.0 (in progress) =========================== +2023-03-13: wsfulton + Improved error checking when using %template to instantiate templates within + the correct scope. + + 1. When a template is instantiated via %template and uses the unary scope + operator ::, an error occurs if the instantiation is attempted within a + namespace that does not enclose the instantiated template. + For example, the following will now error as ::test::max is not enclosed within test1: + + Error: '::test::max' resolves to 'test::max' and was incorrectly instantiated in + scope 'test1' instead of within scope 'test'. + namespace test1 { + %template(maxchar) ::test::max<char>; + } + + 2. SWIG previously failed to always detect a template did not exist when using + %template. In particular when instantiating a global template incorrectly within + namespace. The code below now correctly emits an error: + + Error: Template 'test5::GlobalVector' undefined. + namespace test5 { + } + template<typename T> struct GlobalVector {}; + %template(GVI) test5::GlobalVector<int>; + +2023-03-13: wsfulton + Error out if an attempt is made to define a class using the unary scope + operator ::. The following is not legal C++ and now results in an error: + + Error: Using the unary scope operator :: in class definition '::Space2::B' is invalid. + namespace Space2 { + struct B; + } + struct ::Space2::B {}; + 2023-03-08: wsfulton Fix duplicate const in generated code when template instantiation type is const and use of template parameter is also explicitly const, such as: diff --git a/Examples/test-suite/errors/cpp_class_definition.i b/Examples/test-suite/errors/cpp_class_definition.i index 22d7c1521..d6842eef1 100644 --- a/Examples/test-suite/errors/cpp_class_definition.i +++ b/Examples/test-suite/errors/cpp_class_definition.i @@ -24,3 +24,17 @@ namespace Space2 { }; } +namespace Space2 { + struct B; +} + +struct ::Space2::B { + int val; + B() : val() {} +}; + +struct XX; +// g++: error: global qualification of class name is invalid before ‘{’ token +struct ::XX { + int vvv; +}; diff --git a/Examples/test-suite/errors/cpp_class_definition.stderr b/Examples/test-suite/errors/cpp_class_definition.stderr index 659e254c5..c467bcbcb 100644 --- a/Examples/test-suite/errors/cpp_class_definition.stderr +++ b/Examples/test-suite/errors/cpp_class_definition.stderr @@ -1,3 +1,5 @@ cpp_class_definition.i:11: Warning 302: Identifier 'L' redefined (ignored), cpp_class_definition.i:10: Warning 302: previous definition of 'L'. cpp_class_definition.i:22: Error: 'Space1::A' resolves to 'Space1::A' and was incorrectly instantiated in scope 'Space2' instead of within scope 'Space1'. +cpp_class_definition.i:31: Error: Using the unary scope operator :: in class definition '::Space2::B' is invalid. +cpp_class_definition.i:38: Error: Using the unary scope operator :: in class definition '::XX' is invalid. diff --git a/Examples/test-suite/errors/cpp_invalid_template.stderr b/Examples/test-suite/errors/cpp_invalid_template.stderr index f6bfaaf7d..f39464942 100644 --- a/Examples/test-suite/errors/cpp_invalid_template.stderr +++ b/Examples/test-suite/errors/cpp_invalid_template.stderr @@ -1,3 +1,2 @@ -cpp_invalid_template.i:3: Error: Undefined scope 'SSS' cpp_invalid_template.i:3: Error: Template 'SSS::AAA' undefined. cpp_invalid_template.i:9: Error: 'JJJ' is not defined as a template. (classforward) diff --git a/Examples/test-suite/errors/cpp_namespace_template_bad.i b/Examples/test-suite/errors/cpp_namespace_template_bad.i index f41918f8e..d8a33098e 100644 --- a/Examples/test-suite/errors/cpp_namespace_template_bad.i +++ b/Examples/test-suite/errors/cpp_namespace_template_bad.i @@ -9,6 +9,11 @@ namespace test { }; } +namespace test1 { + %template(maxchar) ::test::max<char>; + %template(vectorchar) ::test::vector<char>; +} + namespace test2 { using namespace test; %template(maxshort) max<short>; @@ -32,9 +37,18 @@ namespace test4 { %template(vectorInteger) vector<Integer>; } -using namespace test; namespace test5 { +// Empty namespace +} +template<typename T> struct GlobalVector { + void gook(T i) {} + void geeko(double d) {} + void geeky(int d) {} +}; +%template(GlobalVectorIntPtr) test5::GlobalVector<int *>; // should fail as GlobalVector is in global namespace + +using namespace test; +namespace test6 { %template(maxdouble) max<double>; %template(vectordouble) vector<double>; } - diff --git a/Examples/test-suite/errors/cpp_namespace_template_bad.stderr b/Examples/test-suite/errors/cpp_namespace_template_bad.stderr index 8ac072d74..d6ee15d5f 100644 --- a/Examples/test-suite/errors/cpp_namespace_template_bad.stderr +++ b/Examples/test-suite/errors/cpp_namespace_template_bad.stderr @@ -1,9 +1,11 @@ -cpp_namespace_template_bad.i:14: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test2' instead of within scope 'test'. -cpp_namespace_template_bad.i:15: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test2' instead of within scope 'test'. -cpp_namespace_template_bad.i:21: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test3' instead of within scope 'test'. -cpp_namespace_template_bad.i:22: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test3' instead of within scope 'test'. -cpp_namespace_template_bad.i:31: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test4' instead of within scope 'test'. -cpp_namespace_template_bad.i:32: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test4' instead of within scope 'test'. -cpp_namespace_template_bad.i:37: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test5' instead of within scope 'test'. -cpp_namespace_template_bad.i:37: Error: No matching function template 'max' found. -cpp_namespace_template_bad.i:38: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test5' instead of within scope 'test'. +cpp_namespace_template_bad.i:13: Error: '::test::max' resolves to 'test::max' and was incorrectly instantiated in scope 'test1' instead of within scope 'test'. +cpp_namespace_template_bad.i:14: Error: '::test::vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test1' instead of within scope 'test'. +cpp_namespace_template_bad.i:19: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test2' instead of within scope 'test'. +cpp_namespace_template_bad.i:20: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test2' instead of within scope 'test'. +cpp_namespace_template_bad.i:26: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test3' instead of within scope 'test'. +cpp_namespace_template_bad.i:27: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test3' instead of within scope 'test'. +cpp_namespace_template_bad.i:36: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test4' instead of within scope 'test'. +cpp_namespace_template_bad.i:37: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test4' instead of within scope 'test'. +cpp_namespace_template_bad.i:48: Error: Template 'test5::GlobalVector' undefined. +cpp_namespace_template_bad.i:52: Error: 'max' resolves to 'test::max' and was incorrectly instantiated in scope 'test6' instead of within scope 'test'. +cpp_namespace_template_bad.i:53: Error: 'vector' resolves to 'test::vector' and was incorrectly instantiated in scope 'test6' instead of within scope 'test'. diff --git a/Examples/test-suite/errors/cpp_nested_template.stderr b/Examples/test-suite/errors/cpp_nested_template.stderr index 363a260f6..5b107cad4 100644 --- a/Examples/test-suite/errors/cpp_nested_template.stderr +++ b/Examples/test-suite/errors/cpp_nested_template.stderr @@ -1,4 +1,2 @@ cpp_nested_template.i:9: Error: 'Temply' resolves to '::Temply' and was incorrectly instantiated in scope 'A' instead of within scope ''. -cpp_nested_template.i:9: Warning 324: Named nested template instantiations not supported. Processing as if no name was given to %template(). cpp_nested_template.i:18: Error: 'Temply' resolves to '::Temply' and was incorrectly instantiated in scope 'B' instead of within scope ''. -cpp_nested_template.i:18: Warning 324: Named nested template instantiations not supported. Processing as if no name was given to %template(). diff --git a/Examples/test-suite/namespace_class.i b/Examples/test-suite/namespace_class.i index cc9940d13..17670a4ac 100644 --- a/Examples/test-suite/namespace_class.i +++ b/Examples/test-suite/namespace_class.i @@ -155,10 +155,8 @@ namespace test %} -namespace test { - - %template(BooT_H) ::BooT<Hello>; -} +using test::Hello; +%template(BooT_H) ::BooT<Hello>; %template(BooT_i) ::BooT<int>; diff --git a/Examples/test-suite/template_partial_specialization.i b/Examples/test-suite/template_partial_specialization.i index fd7c73326..3452535a9 100644 --- a/Examples/test-suite/template_partial_specialization.i +++ b/Examples/test-suite/template_partial_specialization.i @@ -122,24 +122,3 @@ namespace S { namespace S { %template(X2) X<int *>; }; - -#if 0 -// TODO fix: -%inline %{ -namespace Space { -} -template<typename T> struct Vector { - void gook(T i) {} - void geeko(double d) {} - void geeky(int d) {} -}; -/* -template<typename T> struct Vector<T *> { -}; -*/ -%} - -%template(VectorIntPtr) Space::Vector<int *>; // should fail as Vector is in global namespace -// is this a regression - no fails in 1.3.40 too -// Note problem is removed by removing empty Space namespace!! -#endif diff --git a/Source/CParse/parser.y b/Source/CParse/parser.y index b1041c0e7..325856074 100644 --- a/Source/CParse/parser.y +++ b/Source/CParse/parser.y @@ -267,7 +267,7 @@ static String *yyrename = 0; /* Forward renaming operator */ -static String *resolve_create_node_scope(String *cname, int is_class_definition); +static String *resolve_create_node_scope(String *cname, int is_class_definition, int *errored); Hash *Swig_cparse_features(void) { @@ -910,37 +910,45 @@ static Node *nscope_inner = 0; * The scopes required for the symbol name are resolved and/or created, if required. * For example AA::BB::CC as input returns CC and creates the namespace AA then inner * namespace BB in the current scope. */ -static String *resolve_create_node_scope(String *cname, int is_class_definition) { +static String *resolve_create_node_scope(String *cname_in, int is_class_definition, int *errored) { Symtab *gscope = 0; Node *cname_node = 0; + String *cname = cname_in; String *last = Swig_scopename_last(cname); nscope = 0; nscope_inner = 0; + *errored = 0; - if (Strncmp(cname,"::" ,2) != 0) { + if (Strncmp(cname, "::", 2) == 0) { if (is_class_definition) { - /* Only lookup symbols which are in scope via a using declaration but not via a using directive. - For example find y via 'using x::y' but not y via a 'using namespace x'. */ - cname_node = Swig_symbol_clookup_no_inherit(cname, 0); - if (!cname_node) { - Node *full_lookup_node = Swig_symbol_clookup(cname, 0); - if (full_lookup_node) { - /* This finds a symbol brought into scope via both a using directive and a using declaration. */ - Node *last_node = Swig_symbol_clookup_no_inherit(last, 0); - if (last_node == full_lookup_node) - cname_node = last_node; - } + Swig_error(cparse_file, cparse_line, "Using the unary scope operator :: in class definition '%s' is invalid.\n", cname); + *errored = 1; + return last; + } + cname = NewString(Char(cname) + 2); + } + if (is_class_definition) { + /* Only lookup symbols which are in scope via a using declaration but not via a using directive. + For example find y via 'using x::y' but not y via a 'using namespace x'. */ + cname_node = Swig_symbol_clookup_no_inherit(cname, 0); + if (!cname_node) { + Node *full_lookup_node = Swig_symbol_clookup(cname, 0); + if (full_lookup_node) { + /* This finds a symbol brought into scope via both a using directive and a using declaration. */ + Node *last_node = Swig_symbol_clookup_no_inherit(last, 0); + if (last_node == full_lookup_node) + cname_node = last_node; } - } else { - /* For %template, the template needs to be in scope via any means. */ - cname_node = Swig_symbol_clookup(cname, 0); } + } else { + /* For %template, the template needs to be in scope via any means. */ + cname_node = Swig_symbol_clookup(cname, 0); } #if RESOLVE_DEBUG if (!cname_node) - Printf(stdout, "symbol does not yet exist (%d): [%s]\n", is_class_definition, cname); + Printf(stdout, "symbol does not yet exist (%d): [%s]\n", is_class_definition, cname_in); else - Printf(stdout, "symbol does exist (%d): [%s]\n", is_class_definition, cname); + Printf(stdout, "symbol does exist (%d): [%s]\n", is_class_definition, cname_in); #endif if (cname_node) { @@ -1022,8 +1030,8 @@ Printf(stdout, "comparing current: [%s] found: [%s]\n", current_scopename, found if (fail) { String *cname_resolved = NewStringf("%s::%s", found_scopename, last); - Swig_error(cparse_file, cparse_line, "'%s' resolves to '%s' and was incorrectly instantiated in scope '%s' instead of within scope '%s'.\n", cname, cname_resolved, current_scopename, found_scopename); - cname = Copy(last); + Swig_error(cparse_file, cparse_line, "'%s' resolves to '%s' and was incorrectly instantiated in scope '%s' instead of within scope '%s'.\n", cname_in, cname_resolved, current_scopename, found_scopename); + *errored = 1; Delete(cname_resolved); } } @@ -1032,22 +1040,17 @@ Printf(stdout, "comparing current: [%s] found: [%s]\n", current_scopename, found Delete(found_scopename); } } else if (!is_class_definition) { - /* A template instantiation requires a template to be found in scope... fail here too? - Swig_error(cparse_file, cparse_line, "No template found to instantiate '%s' with %%template.\n", cname); - */ + /* A template instantiation requires a template to be found in scope */ + Swig_error(cparse_file, cparse_line, "Template '%s' undefined.\n", cname_in); + *errored = 1; } - if (Swig_scopename_check(cname)) { + if (*errored) + return last; + + if (Swig_scopename_check(cname) && !*errored) { Node *ns; String *prefix = Swig_scopename_prefix(cname); - if (prefix && (Strncmp(prefix,"::",2) == 0)) { -/* I don't think we can use :: global scope to declare classes and hence neither %template. - consider reporting error instead - wsfulton. */ - /* Use the global scope */ - String *nprefix = NewString(Char(prefix)+2); - Delete(prefix); - prefix= nprefix; - gscope = set_scope_to_global(); - } if (Len(prefix) == 0) { String *base = Copy(last); /* Use the global scope, but we need to add a 'global' namespace. */ @@ -1065,10 +1068,12 @@ Printf(stdout, "comparing current: [%s] found: [%s]\n", current_scopename, found ns = Swig_symbol_clookup(prefix,0); if (!ns) { Swig_error(cparse_file,cparse_line,"Undefined scope '%s'\n", prefix); + *errored = 1; } else { Symtab *nstab = Getattr(ns,"symtab"); if (!nstab) { Swig_error(cparse_file,cparse_line, "'%s' is not defined as a valid scope.\n", prefix); + *errored = 1; ns = 0; } else { /* Check if the node scope is the current scope */ @@ -2814,30 +2819,32 @@ types_directive : TYPES LPAREN parms RPAREN stringbracesemi { template_directive: SWIGTEMPLATE LPAREN idstringopt RPAREN idcolonnt LESSTHAN valparms GREATERTHAN SEMI { Parm *p; - Node *n; + Node *n = 0; Node *outer_class = currentOuterClass; Symtab *tscope = 0; String *symname = $3 ? NewString($3) : 0; + int errored_flag = 0; $$ = 0; tscope = Swig_symbol_current(); /* Get the current scope */ /* If the class name is qualified, we need to create or lookup namespace entries */ - $5 = resolve_create_node_scope($5, 0); + $5 = resolve_create_node_scope($5, 0, &errored_flag); - if (nscope_inner && Strcmp(nodeType(nscope_inner), "class") == 0) { - outer_class = nscope_inner; - } + if (!errored_flag) { + if (nscope_inner && Strcmp(nodeType(nscope_inner), "class") == 0) + outer_class = nscope_inner; - /* - We use the new namespace entry 'nscope' only to - emit the template node. The template parameters are - resolved in the current 'tscope'. + /* + We use the new namespace entry 'nscope' only to + emit the template node. The template parameters are + resolved in the current 'tscope'. - This is closer to the C++ (typedef) behavior. - */ - n = Swig_cparse_template_locate($5, $7, symname, tscope); + This is closer to the C++ (typedef) behavior. + */ + n = Swig_cparse_template_locate($5, $7, symname, tscope); + } /* Patch the argument types to respect namespaces */ p = $7; @@ -3647,6 +3654,7 @@ cpp_class_decl: storage_class cpptype idcolon class_virt_specifier_opt inherit L String *prefix; List *bases = 0; Node *scope = 0; + int errored_flag = 0; String *code; $<node>$ = new_node("class"); Setline($<node>$,cparse_start_line); @@ -3662,7 +3670,7 @@ cpp_class_decl: storage_class cpptype idcolon class_virt_specifier_opt inherit L Setattr($<node>$,"prev_symtab",Swig_symbol_current()); /* If the class name is qualified. We need to create or lookup namespace/scope entries */ - scope = resolve_create_node_scope($3, 1); + scope = resolve_create_node_scope($3, 1, &errored_flag); /* save nscope_inner to the class - it may be overwritten in nested classes*/ Setattr($<node>$, "nested:innerscope", nscope_inner); Setattr($<node>$, "nested:nscope", nscope); |