Detect some cases of reading uninitialized private variables

Cython/Compiler/Nodes.py

@@ -5890,6 +5890,8 @@ class ParallelStatNode(StatNode, ParallelNode):
 
     def analyse_expressions(self, env):
         self.body.analyse_expressions(env)
+        self.analyse_sharing_attributes(env)
+        self.check_use_unitialized_privates()
 
     def analyse_sharing_attributes(self, env):
         """
@@ -5995,6 +5997,31 @@ class ParallelStatNode(StatNode, ParallelNode):
         code.putln("%s = %s;" % (cname, entry.cname))
         entry.cname = cname
 
+    def check_use_unitialized_privates(self):
+        """
+        This checks for uninitialized private variables, it's far from
+        fool-proof as it does not take control flow into account, nor
+        assignment to a variable as both the lhs and rhs. So it detects only
+        cases like this:
+
+            for i in prange(10, nogil=True):
+                var = x # error, x is private and read before assigned
+                x = i
+        """
+        from Cython.Compiler import ParseTreeTransforms
+
+        transform = ParseTreeTransforms.FindUninitializedParallelVars()
+        transform(self.body)
+
+        for entry, pos in transform.used_vars:
+            if entry in self.privates:
+                assignment_pos, op = self.assignments[entry]
+
+                # Reading reduction variables is valid (in fact necessary)
+                # before assignment
+                if not op and pos < assignment_pos:
+                    error(pos, "Private variable referenced before assignment")
+
     def declare_closure_privates(self, code):
         """
         Set self.privates to a dict mapping C variable names that are to be
@@ -6032,10 +6059,6 @@ class ParallelWithBlockNode(ParallelStatNode):
 
     nogil_check = None
 
-    def analyse_expressions(self, env):
-        super(ParallelWithBlockNode, self).analyse_expressions(env)
-        self.analyse_sharing_attributes(env)
-
     def generate_execution_code(self, code):
         self.declare_closure_privates(code)
 
@@ -6156,7 +6179,6 @@ class ParallelRangeNode(ParallelStatNode):
                 self.index_type = PyrexTypes.widest_numeric_type(
                                         self.index_type, node.type)
 
-        super(ParallelRangeNode, self).analyse_expressions(env)
         if self.else_clause is not None:
             self.else_clause.analyse_expressions(env)
 
@@ -6169,6 +6191,7 @@ class ParallelRangeNode(ParallelStatNode):
             self.assignments[self.target.entry] = self.target.pos, None
 
         self.analyse_sharing_attributes(env)
+        super(ParallelRangeNode, self).analyse_expressions(env)
 
     def nogil_check(self, env):
         names = 'start', 'stop', 'step', 'target'

Cython/Compiler/ParseTreeTransforms.py

@@ -2273,6 +2273,24 @@ class TransformBuiltinMethods(EnvTransform):
         return node
 
 
+class FindUninitializedParallelVars(CythonTransform, SkipDeclarations):
+    """
+    This transform isn't part of the pipeline, it simply finds all references
+    to variables in parallel blocks.
+    """
+
+    def __init__(self):
+        CythonTransform.__init__(self, None)
+        self.used_vars = []
+
+    def visit_ParallelStatNode(self, node):
+        return node
+
+    def visit_NameNode(self, node):
+        self.used_vars.append((node.entry, node.pos))
+        return node
+
+
 class DebugTransform(CythonTransform):
     """
     Write debug information for this Cython module.

Cython/Compiler/TypeInference.py

@@ -39,7 +39,24 @@ class MarkAssignments(CythonTransform):
 
             if self.parallel_block_stack:
                 parallel_node = self.parallel_block_stack[-1]
-                parallel_node.assignments[lhs.entry] = (lhs.pos, inplace_op)
+                previous_assignment = parallel_node.assignments.get(lhs.entry)
+
+                # If there was a previous assignment to the variable, keep the
+                # previous assignment position
+                if previous_assignment:
+                    pos, previous_inplace_op = previous_assignment
+
+                    if (inplace_op and previous_inplace_op and
+                            inplace_op != previous_inplace_op):
+                        # x += y; x *= y
+                        t = (inplace_op, previous_inplace_op)
+                        error(lhs.pos,
+                              "Reduction operator '%s' is inconsistent "
+                              "with previous reduction operator '%s'" % t)
+                else:
+                    pos = lhs.pos
+
+                parallel_node.assignments[lhs.entry] = (pos, inplace_op)
 
         elif isinstance(lhs, ExprNodes.SequenceNode):
             for arg in lhs.args:

tests/errors/e_cython_parallel.pyx

@@ -39,6 +39,26 @@ with nogil, cython.parallel.parallel():
 with nogil, cython.parallel.parallel:
     pass
 
+cdef int y
+# this is not valid
+for i in prange(10, nogil=True):
+    i = y * 4
+    y = i
+
+# this is valid
+for i in prange(10, nogil=True):
+    y = i
+    i = y * 4
+    y = i
+
+with nogil, cython.parallel.parallel():
+    i = y
+    y = i
+
+for i in prange(10, nogil=True):
+    y += i
+    y *= i
+
 _ERRORS = u"""
 e_cython_parallel.pyx:3:8: cython.parallel.parallel is not a module
 e_cython_parallel.pyx:4:0: No such directive: cython.parallel.something
@@ -53,4 +73,7 @@ e_cython_parallel.pyx:30:9: Can only iterate over an iteration variable
 e_cython_parallel.pyx:33:10: Must be of numeric type, not int *
 e_cython_parallel.pyx:36:33: Closely nested 'with parallel:' blocks are disallowed
 e_cython_parallel.pyx:39:12: The parallel directive must be called
+e_cython_parallel.pyx:45:10: Private variable referenced before assignment
+e_cython_parallel.pyx:55:9: Private variable referenced before assignment
+e_cython_parallel.pyx:60:6: Reduction operator '*' is inconsistent with previous reduction operator '+'
 """