Merge branch 'hotfix/v1.8.6'

Author: iMichka

CHANGELOG.md

@@ -1,6 +1,13 @@
 Changes
 =======
 
+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
 -------------
 

pygccxml/__init__.py

@@ -40,4 +40,4 @@
 # TODO:
 # 1. Add "explicit" property for constructors
 
-__version__ = '1.8.5'
+__version__ = '1.8.6'

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):

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)

unittests/data/test_non_copyable_recursive.hpp

@@ -0,0 +1,52 @@
+// 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 <sstream>
+
+// 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 {
+  public:
+    FileStreamDataStream(const std::istream* s) {}
+
+  protected:
+    std::istream* mInStream;
+  };
+}
+

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:

unittests/test_non_copyable_recursive.py

@@ -0,0 +1,77 @@
+# 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_non_copyable_recursive.hpp"
+
+    def test_infinite_recursion_base_classes(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)
+
+        # 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)
+        self.assertTrue(declarations.type_traits_classes.is_noncopyable(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)
+        self.assertFalse(declarations.type_traits_classes.is_noncopyable(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()