From d0f52286b4ec66a613e9891f6b60ee2d890a2f7e Mon Sep 17 00:00:00 2001 From: Matt McCormick Date: Sun, 26 Feb 2017 10:53:19 -0500 Subject: [PATCH 01/11] Fix _HAS_TR1=0 definition for msvc9 This can get expanded the wrong way with VS9 leading to a compile error. Cherry-picked from the develop branch --- pygccxml/parser/source_reader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygccxml/parser/source_reader.py b/pygccxml/parser/source_reader.py index 36bc16c0..eafb4aa9 100644 --- a/pygccxml/parser/source_reader.py +++ b/pygccxml/parser/source_reader.py @@ -183,7 +183,7 @@ def __create_command_line_castxml(self, source_file, xmlfile): cmd.append('--castxml-cc-msvc ' + '"%s"' % self.__config.compiler_path) if 'msvc9' == self.__config.compiler: - cmd.append('-D"_HAS_TR1=0"') + cmd.append('"-D_HAS_TR1=0"') else: # On mac or linux, use gcc or clang (the flag is the same) From 1ef0590832a093df65955b6ce4b3446f4b76d398 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Sun, 26 Feb 2017 20:05:19 +0100 Subject: [PATCH 02/11] Update changelog for v1.8.6 --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5bd91075..b86f0ebc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,11 @@ Changes ======= +Version 1.8.6 +------------- + +1. Fix _HAS_TR1=0 definition for msvc9 (#72) + Version 1.8.5 ------------- From 6929f36f2743d870e3bf5d2b56e9920785fc70fa Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Sun, 26 Feb 2017 20:17:36 +0100 Subject: [PATCH 03/11] Add test to demonstrate error with find_noncopyable_vars (#71) The test is not run by the test suite for the moment. --- unittests/data/test_stream.hpp | 18 +++++++++++++ unittests/test_stream.py | 47 ++++++++++++++++++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 unittests/data/test_stream.hpp create mode 100644 unittests/test_stream.py diff --git a/unittests/data/test_stream.hpp b/unittests/data/test_stream.hpp new file mode 100644 index 00000000..58d55584 --- /dev/null +++ b/unittests/data/test_stream.hpp @@ -0,0 +1,18 @@ +// Copyright 2014-2016 Insight Software Consortium. +// Copyright 2004-2008 Roman Yakovenko. +// Distributed under the Boost Software License, Version 1.0. +// See http://www.boost.org/LICENSE_1_0.txt + +#include + +namespace Test2 { + +class FileStreamDataStream { + public: + FileStreamDataStream(const std::istream* s) {} + + protected: + std::istream* mInStream; + }; +} + diff --git a/unittests/test_stream.py b/unittests/test_stream.py new file mode 100644 index 00000000..8d3f7595 --- /dev/null +++ b/unittests/test_stream.py @@ -0,0 +1,47 @@ +# Copyright 2014-2016 Insight Software Consortium. +# Copyright 2004-2009 Roman Yakovenko. +# Distributed under the Boost Software License, Version 1.0. +# See http://www.boost.org/LICENSE_1_0.txt + +import unittest +import parser_test_case + +from pygccxml import parser +from pygccxml import declarations + + +class Test(parser_test_case.parser_test_case_t): + + def __init__(self, *args): + parser_test_case.parser_test_case_t.__init__(self, *args) + self.header = "test_stream.hpp" + self.config.cflags = "-std=c++11" + + def test_stream_non_copyable(self): + """ + Test find_noncopyable_vars + + See #71 + + find_noncopyable_vars is throwing: + RuntimeError: maximum recursion depth exceeded while + calling a Python object + """ + decls = parser.parse([self.header], self.config) + global_ns = declarations.get_global_namespace(decls) + + test_ns = global_ns.namespace('Test2') + cls = test_ns.class_('FileStreamDataStream') + declarations.type_traits_classes.find_noncopyable_vars(cls) + +def create_suite(): + suite = unittest.TestSuite() + suite.addTest(unittest.makeSuite(Test)) + return suite + + +def run_suite(): + unittest.TextTestRunner(verbosity=2).run(create_suite()) + +if __name__ == "__main__": + run_suite() From 20ed93b5cfd2bd7a26ad0fdc2a9cb942196cefbc Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Mon, 27 Feb 2017 22:55:58 +0100 Subject: [PATCH 04/11] Extend test for infinite recursion bug in find_non_copyable_vars (#71) These tests are still disabled as they are is still failing, but they now clearly show the problem. --- unittests/data/test_stream.hpp | 34 ++++++++++++++++++++++++++++++++++ unittests/test_stream.py | 33 +++++++++++++++++++++++++++++++-- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/unittests/data/test_stream.hpp b/unittests/data/test_stream.hpp index 58d55584..0e4e5149 100644 --- a/unittests/data/test_stream.hpp +++ b/unittests/data/test_stream.hpp @@ -5,6 +5,40 @@ #include +// Demonstration of the real problem with basic c++ +namespace Test1 { + +// Forward declaration +class Base2; + +// Base 1, with pointer to Base2 +class Base1 { + private: + Base1(); + + protected: + Base2* aBasePtr2; +}; + +// Base 2, child class of Base1 +class Base2: public Base1 { + private: + Base2(); +}; + +// Child class of Base2 +// Holds a pointer to Base2 +class Child: public Base2 { + private: + Child(); + + protected: + Base2* aBasePtr2; +}; + +} + +// Real-life test with std::istream where this happened namespace Test2 { class FileStreamDataStream { diff --git a/unittests/test_stream.py b/unittests/test_stream.py index 8d3f7595..e1de32f7 100644 --- a/unittests/test_stream.py +++ b/unittests/test_stream.py @@ -17,23 +17,52 @@ def __init__(self, *args): self.header = "test_stream.hpp" self.config.cflags = "-std=c++11" - def test_stream_non_copyable(self): + def test_infinite_recursion_base_classes(self): """ Test find_noncopyable_vars See #71 - find_noncopyable_vars is throwing: + find_noncopyable_vars was throwing: RuntimeError: maximum recursion depth exceeded while calling a Python object """ decls = parser.parse([self.header], self.config) global_ns = declarations.get_global_namespace(decls) + # Description of the problem (before the fix): + # find_noncopyable_vars (on Child class) looks up the variables, + # and finds aBasePtr2 (a pointer to the Base2 class). + # Then it looks recursively at the base classes of Base2, and finds + # Base1. Then, it looks up the variables from Base, to check if Base1 + # is non copyable. It finds another aBasePtr2 variable, which leads to + # a new check of Base2; this recurses infinitely. + test_ns = global_ns.namespace('Test1') + cls = test_ns.class_('Child') + declarations.type_traits_classes.find_noncopyable_vars(cls) + + def test_infinite_recursion_sstream(self): + """ + Test find_noncopyable_vars + + See #71 + + find_noncopyable_vars was throwing: + RuntimeError: maximum recursion depth exceeded while + calling a Python object + """ + decls = parser.parse([self.header], self.config) + global_ns = declarations.get_global_namespace(decls) + + # Real life example of the bug. This leads to a similar error, + # but the situation is more complex as there are multiple + # classes that are related the one to the others + # (basic_istream, basic_ios, ios_base, ...) test_ns = global_ns.namespace('Test2') cls = test_ns.class_('FileStreamDataStream') declarations.type_traits_classes.find_noncopyable_vars(cls) + def create_suite(): suite = unittest.TestSuite() suite.addTest(unittest.makeSuite(Test)) From 8c86ad867cb625b990951a734877f8d6ac205e60 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Tue, 28 Feb 2017 22:26:00 +0100 Subject: [PATCH 05/11] Rename test --- .../data/{test_stream.hpp => test_non_copyable_recursive.hpp} | 0 unittests/{test_stream.py => test_non_copyable_recursive.py} | 2 +- 2 files changed, 1 insertion(+), 1 deletion(-) rename unittests/data/{test_stream.hpp => test_non_copyable_recursive.hpp} (100%) rename unittests/{test_stream.py => test_non_copyable_recursive.py} (97%) diff --git a/unittests/data/test_stream.hpp b/unittests/data/test_non_copyable_recursive.hpp similarity index 100% rename from unittests/data/test_stream.hpp rename to unittests/data/test_non_copyable_recursive.hpp diff --git a/unittests/test_stream.py b/unittests/test_non_copyable_recursive.py similarity index 97% rename from unittests/test_stream.py rename to unittests/test_non_copyable_recursive.py index e1de32f7..2ff327c7 100644 --- a/unittests/test_stream.py +++ b/unittests/test_non_copyable_recursive.py @@ -14,7 +14,7 @@ class Test(parser_test_case.parser_test_case_t): def __init__(self, *args): parser_test_case.parser_test_case_t.__init__(self, *args) - self.header = "test_stream.hpp" + self.header = "test_non_copyable_recursive.hpp" self.config.cflags = "-std=c++11" def test_infinite_recursion_base_classes(self): From 1beb856fa0ebcc3a32903878153dc2721f10d538 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Tue, 28 Feb 2017 22:27:10 +0100 Subject: [PATCH 06/11] Remove c++11 flag, the failure is not related to the c++ standard --- unittests/test_non_copyable_recursive.py | 1 - 1 file changed, 1 deletion(-) diff --git a/unittests/test_non_copyable_recursive.py b/unittests/test_non_copyable_recursive.py index 2ff327c7..225bb8a3 100644 --- a/unittests/test_non_copyable_recursive.py +++ b/unittests/test_non_copyable_recursive.py @@ -15,7 +15,6 @@ class Test(parser_test_case.parser_test_case_t): def __init__(self, *args): parser_test_case.parser_test_case_t.__init__(self, *args) self.header = "test_non_copyable_recursive.hpp" - self.config.cflags = "-std=c++11" def test_infinite_recursion_base_classes(self): """ From aa23b40cafd7a52f74712d95e3c80e65d74cf95f Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Tue, 28 Feb 2017 22:58:05 +0100 Subject: [PATCH 07/11] Fix possible infinite recursion in find_noncopyable_vars() (#71) To fix this bug, a list of already parsed variables is passed trhough the different methods. This is optional. In general, if you call is_noncopyable, you do not need to provide the list --- pygccxml/declarations/type_traits_classes.py | 69 +++++++++++++++++--- 1 file changed, 59 insertions(+), 10 deletions(-) diff --git a/pygccxml/declarations/type_traits_classes.py b/pygccxml/declarations/type_traits_classes.py index ab3f2d7b..bc2bf654 100644 --- a/pygccxml/declarations/type_traits_classes.py +++ b/pygccxml/declarations/type_traits_classes.py @@ -152,12 +152,18 @@ def find_copy_constructor(type_): return None -def find_noncopyable_vars(type_): +def find_noncopyable_vars(type_, already_visited_cls_vars=None): """ Returns list of all `noncopyable` variables. + If an already_visited_cls_vars list is provided as argument, the returned + list will not contain these variables. This list will be extended with + whatever variables pointing to classes have been found. + Args: type_ (declarations.class_t): the class to be searched. + already_visited_cls_vars (list): optional list of vars that should not + be checked a second time, to prevent infinite recursions. Returns: list: list of all `noncopyable` variables. @@ -172,6 +178,9 @@ def find_noncopyable_vars(type_): allow_empty=True) noncopyable_vars = [] + if already_visited_cls_vars is None: + already_visited_cls_vars = [] + message = ( "__contains_noncopyable_mem_var - %s - TRUE - " + "contains const member variable") @@ -196,7 +205,13 @@ def find_noncopyable_vars(type_): if class_traits.is_my_case(type_): cls = class_traits.get_declaration(type_) - if is_noncopyable(cls): + + # Exclude classes that have already been visited. + if cls in already_visited_cls_vars: + continue + already_visited_cls_vars.append(cls) + + if is_noncopyable(cls, already_visited_cls_vars): logger.debug((message + " - class that is not copyable") % type_.decl_string) noncopyable_vars.append(mvar) @@ -632,8 +647,20 @@ def is_convertible(source, target): return __is_convertible_t(source, target).is_convertible() -def __is_noncopyable_single(class_): - """implementation details""" +def __is_noncopyable_single(class_, already_visited_cls_vars=None): + """ + Implementation detail. + + Checks if the class is non copyable, without considering the base classes. + + Args: + class_ (declarations.class_t): the class to be checked + already_visited_cls_vars (list): optional list of vars that should not + be checked a second time, to prevent infinite recursions. + + Returns: + bool: if the class is non copyable + """ # It is not enough to check base classes, we should also to check # member variables. logger = utils.loggers.cxx_parser @@ -650,7 +677,11 @@ def __is_noncopyable_single(class_): " public destructor: yes"]) logger.debug(msg) return False - if find_noncopyable_vars(class_): + + if already_visited_cls_vars is None: + already_visited_cls_vars = [] + + if find_noncopyable_vars(class_, already_visited_cls_vars): logger.debug( ("__is_noncopyable_single(TRUE) - %s - contains noncopyable " + "members") % class_.decl_string) @@ -662,9 +693,22 @@ def __is_noncopyable_single(class_): return False -def is_noncopyable(class_): - """returns True, if class is noncopyable, False otherwise""" +def is_noncopyable(class_, already_visited_cls_vars=None): + """ + Checks if class is non copyable + + Args: + class_ (declarations.class_t): the class to be checked + already_visited_cls_vars (list): optional list of vars that should not + be checked a second time, to prevent infinite recursions. + In general you can ignore this argument, it is mainly used during + recursive calls of is_noncopyable() done by pygccxml. + + Returns: + bool: if the class is non copyable + """ logger = utils.loggers.cxx_parser + class_decl = class_traits.get_declaration(class_) true_header = "is_noncopyable(TRUE) - %s - " % class_.decl_string @@ -683,6 +727,9 @@ def is_noncopyable(class_): if copy_ and copy_.access_type == 'public' and not copy_.is_artificial: return False + if already_visited_cls_vars is None: + already_visited_cls_vars = [] + for base_desc in class_decl.recursive_bases: assert isinstance(base_desc, class_declaration.hierarchy_info_t) @@ -700,13 +747,15 @@ def is_noncopyable(class_): true_header + "there is private copy constructor") return True - elif __is_noncopyable_single(base_desc.related_class): + elif __is_noncopyable_single( + base_desc.related_class, already_visited_cls_vars): logger.debug( true_header + "__is_noncopyable_single returned True") return True - if __is_noncopyable_single(base_desc.related_class): + if __is_noncopyable_single( + base_desc.related_class, already_visited_cls_vars): logger.debug( true_header + "__is_noncopyable_single returned True") @@ -722,7 +771,7 @@ def is_noncopyable(class_): logger.debug(true_header + "has private destructor") return True else: - return __is_noncopyable_single(class_decl) + return __is_noncopyable_single(class_decl, already_visited_cls_vars) def is_unary_operator(oper): From a1350d8375753208ac4a0279316f91dc6621c1e8 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Tue, 28 Feb 2017 23:09:03 +0100 Subject: [PATCH 08/11] Add test_non_copyable_recursive to test suite --- unittests/test_all.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/unittests/test_all.py b/unittests/test_all.py index 57eb1a86..1b91077c 100644 --- a/unittests/test_all.py +++ b/unittests/test_all.py @@ -76,6 +76,7 @@ import test_function_pointer import test_directory_cache import test_config +import test_non_copyable_recursive testers = [ # , demangled_tester # failing right now @@ -144,7 +145,8 @@ test_pattern_parser, test_function_pointer, test_directory_cache, - test_config + test_config, + test_non_copyable_recursive ] if 'posix' in os.name: From 761133f78b82e3d4bf7065a6e625382ffc7e6c02 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Tue, 28 Feb 2017 23:24:13 +0100 Subject: [PATCH 09/11] Add is_noncopyable checks to the new test --- unittests/test_non_copyable_recursive.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/unittests/test_non_copyable_recursive.py b/unittests/test_non_copyable_recursive.py index 225bb8a3..8ae4fb9d 100644 --- a/unittests/test_non_copyable_recursive.py +++ b/unittests/test_non_copyable_recursive.py @@ -39,6 +39,7 @@ def test_infinite_recursion_base_classes(self): test_ns = global_ns.namespace('Test1') cls = test_ns.class_('Child') declarations.type_traits_classes.find_noncopyable_vars(cls) + self.assertTrue(declarations.type_traits_classes.is_noncopyable(cls)) def test_infinite_recursion_sstream(self): """ @@ -60,6 +61,7 @@ def test_infinite_recursion_sstream(self): test_ns = global_ns.namespace('Test2') cls = test_ns.class_('FileStreamDataStream') declarations.type_traits_classes.find_noncopyable_vars(cls) + self.assertFalse(declarations.type_traits_classes.is_noncopyable(cls)) def create_suite(): From 59397b98ca4c2756409d13725ead917b59d69e85 Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Sat, 4 Mar 2017 15:02:50 +0100 Subject: [PATCH 10/11] Document changes for version 1.8.6 --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index b86f0ebc..2d52e9a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ Version 1.8.6 1. Fix _HAS_TR1=0 definition for msvc9 (#72) +2. Fix possible infinite recursion in ```find_noncopyable_vars()``` (#71) + Version 1.8.5 ------------- From fab1af3f91bb3e73fdda18ce928c09d2b37d3c2d Mon Sep 17 00:00:00 2001 From: Michka Popoff Date: Sat, 4 Mar 2017 15:03:22 +0100 Subject: [PATCH 11/11] Bump version number to 1.8.6 --- pygccxml/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pygccxml/__init__.py b/pygccxml/__init__.py index c1b97c2c..67696885 100644 --- a/pygccxml/__init__.py +++ b/pygccxml/__init__.py @@ -40,4 +40,4 @@ # TODO: # 1. Add "explicit" property for constructors -__version__ = '1.8.5' +__version__ = '1.8.6'