Daniel Harding wrote:
Thanks for taking my changes so quickly!  However, I realized today that
they weren't completely correct.  Dict comprehensions and set
comprehensions should create their own scope, just like generator
expressions.  I didn't account for this in my changes, so I have
attached a patch against astng that makes DictComp and SetComp subclass
from LocalsDictNodeNG rather than NodeNG.  I also moved their
definitions from node_classes.py to scoped_nodes.py.

Once that change was made, I had to make a change to VariableChecker in
pylint to prevent spurious E0602 undefined variable errors.  I added
visit_*/leave_* methods for dict and set comprehensions which mirror the
visit_genexpr/leave_genexpr methods.  I also changed the scope type used
in visit_genexpr to be 'comprehension' and then reused that for dict and
set comprehensions.  Attached is a second patch with these changes
against pylint.

Because both of my patches mentioned above had a fair amount of duplicated code, I refactored them to move the common code into a new base clase (in the case of astng) or into common functions (in the case of pylint). The attached patches are functionally equivalent to the ones mentioned above, but in my opinion are a little bit cleaner.

Cheers,

-Daniel
# HG changeset patch
# User Daniel Harding <[email protected]>
# Date 1286802279 0
# Node ID 5ab1fc35fbe8360c934d2ea1a21163f70bda88b6
# Parent  12e5f7eefe73dff220e4d17b93aa42eabbc4d8d4
make dictionary comprehensions and set comprehensions have their own scope, 
like generator expressions

diff -r 12e5f7eefe73 -r 5ab1fc35fbe8 node_classes.py
--- a/node_classes.py   Thu Oct 07 18:46:23 2010 +0200
+++ b/node_classes.py   Mon Oct 11 13:04:39 2010 +0000
@@ -505,14 +505,6 @@
         raise IndexError(key)
 
 
-class DictComp(NodeNG):
-    """class representing a DictComp node"""
-    _astng_fields = ('key', 'value', 'generators')
-    key = None
-    value = None
-    generators = None
-
-
 class Discard(StmtMixIn, NodeNG):
     """class representing a Discard node"""
     _astng_fields = ('value',)
@@ -709,13 +701,6 @@
         return self.elts
 
 
-class SetComp(NodeNG):
-    """class representing a SetComp node"""
-    _astng_fields = ('elt', 'generators')
-    elt = None
-    generators = None
-
-
 class Slice(NodeNG):
     """class representing a Slice node"""
     _astng_fields = ('lower', 'upper', 'step')
diff -r 12e5f7eefe73 -r 5ab1fc35fbe8 nodes.py
--- a/nodes.py  Thu Oct 07 18:46:23 2010 +0200
+++ b/nodes.py  Mon Oct 11 13:04:39 2010 +0000
@@ -54,12 +54,13 @@
 from logilab.astng.node_classes import Arguments, AssAttr, Assert, Assign, \
     AssName, AugAssign, Backquote, BinOp, BoolOp, Break, CallFunc, Compare, \
     Comprehension, Const, Continue, Decorators, DelAttr, DelName, Delete, \
-    Dict, DictComp, Discard, Ellipsis, EmptyNode, ExceptHandler, Exec, \
-    ExtSlice, For, From, Getattr, Global, If, IfExp, Import, Index, Keyword, \
-    List, ListComp, Name, Pass, Print, Raise, Return, Set, SetComp, Slice, \
-    Subscript, TryExcept, TryFinally, Tuple, UnaryOp, While, With, Yield, \
+    Dict, Discard, Ellipsis, EmptyNode, ExceptHandler, Exec, ExtSlice, For, \
+    From, Getattr, Global, If, IfExp, Import, Index, Keyword, \
+    List, ListComp, Name, Pass, Print, Raise, Return, Set, Slice, Subscript, \
+    TryExcept, TryFinally, Tuple, UnaryOp, While, With, Yield, \
     const_factory
-from logilab.astng.scoped_nodes import Module, GenExpr, Lambda, Function, Class
+from logilab.astng.scoped_nodes import Module, GenExpr, Lambda, DictComp, \
+    SetComp, Function, Class
 
 ALL_NODE_CLASSES = (
     Arguments, AssAttr, Assert, Assign, AssName, AugAssign,
diff -r 12e5f7eefe73 -r 5ab1fc35fbe8 scoped_nodes.py
--- a/scoped_nodes.py   Thu Oct 07 18:46:23 2010 +0200
+++ b/scoped_nodes.py   Mon Oct 11 13:04:39 2010 +0000
@@ -19,7 +19,7 @@
 # with logilab-astng. If not, see <http://www.gnu.org/licenses/>.
 """This module contains the classes for "scoped" node, i.e. which are opening a
 new local scope in the language definition : Module, Class, Function (and
-Lambda and GenExpr to some extends).
+Lambda, GenExpr, DictComp and SetComp to some extent).
 
 """
 from __future__ import generators
@@ -118,7 +118,7 @@
 
     def scope(self):
         """return the first node defining a new scope (i.e. Module,
-        Function, Class, Lambda but also GenExpr)
+        Function, Class, Lambda but also GenExpr, DictComp and SetComp)
         """
         return self
 
@@ -402,7 +402,14 @@
             return [name for name in self.keys() if not name.startswith('_')]
 
 
-class GenExpr(LocalsDictNodeNG):
+class ScopedComprehensionNodeNG(LocalsDictNodeNG):
+    def frame(self):
+        return self.parent.frame()
+
+    scope_lookup = LocalsDictNodeNG._scope_lookup
+
+
+class GenExpr(ScopedComprehensionNodeNG):
     _astng_fields = ('elt', 'generators')
 
     def __init__(self):
@@ -410,9 +417,24 @@
         self.elt = None
         self.generators = []
 
-    def frame(self):
-        return self.parent.frame()
-GenExpr.scope_lookup = LocalsDictNodeNG._scope_lookup
+
+class DictComp(ScopedComprehensionNodeNG):
+    _astng_fields = ('key', 'value', 'generators')
+
+    def __init__(self):
+        self.locals = {}
+        self.key = None
+        self.value = None
+        self.generators = []
+
+
+class SetComp(ScopedComprehensionNodeNG):
+    _astng_fields = ('elt', 'generators')
+
+    def __init__(self):
+        self.locals = {}
+        self.elt = None
+        self.generators = []
 
 
 # Function  ###################################################################
# HG changeset patch
# User Daniel Harding <[email protected]>
# Date 1286801891 0
# Node ID a7e5bf60a140fce2037929b407853d77bba6adb4
# Parent  1fe7a99b148f9b5305b6b079161fb265b9ae5b65
update VariablesChecker to take into account the nested scopes of dictionary 
and set comprehensions

diff -r 1fe7a99b148f -r a7e5bf60a140 checkers/variables.py
--- a/checkers/variables.py     Mon Oct 11 12:57:58 2010 +0000
+++ b/checkers/variables.py     Mon Oct 11 12:58:11 2010 +0000
@@ -178,17 +178,25 @@
         # do not check for not used locals here
         self._to_consume.pop()
 
-    def visit_genexpr(self, node):
-        """visit genexpr: update consumption analysis variable
-        """
-        self._to_consume.append((copy(node.locals), {}, 'genexpr'))
-
-    def leave_genexpr(self, _):
-        """leave genexpr: update consumption analysis variable
+    def _visit_scoped_comprehension(self, node):
+        """visit genexpr/dictcomp/setcomp: update consumption analysis variable
+        """
+        self._to_consume.append((copy(node.locals), {}, 'comprehension'))
+
+    visit_genexpr = _visit_scoped_comprehension
+    visit_dictcomp = _visit_scoped_comprehension
+    visit_setcomp = _visit_scoped_comprehension
+
+    def _leave_scoped_comprehension(self, _):
+        """leave genexpr/dictcomp/setcomp: update consumption analysis variable
         """
         # do not check for not used locals here
         self._to_consume.pop()
 
+    leave_genexpr = _leave_scoped_comprehension
+    leave_dictcomp = _leave_scoped_comprehension
+    leave_setcomp = _leave_scoped_comprehension
+
     def visit_function(self, node):
         """visit function: update consumption analysis variable and check 
locals
         """
@@ -349,9 +357,9 @@
             # scope, ignore it. This prevents to access this scope instead of
             # the globals one in function members when there are some common
             # names. The only exception is when the starting scope is a
-            # genexpr and its direct outer scope is a class
+            # comprehension and its direct outer scope is a class
             if scope_type == 'class' and i != start_index and not (
-                base_scope_type == 'genexpr' and i == start_index-1):
+                base_scope_type == 'comprehension' and i == start_index-1):
                 # XXX find a way to handle class scope in a smoother way
                 continue
             # the name has already been consumed, only check it's not a loop
_______________________________________________
Python-Projects mailing list
[email protected]
http://lists.logilab.org/mailman/listinfo/python-projects

Reply via email to