Possible bug in evaluating code blocks

Hi all,
*whew* This one has had my stumped for a while.

I have a Genshi template for Trac that I've been working on
that
contains some rendering-related functions in a <?python
?> code block.
 Under version 0.4 of Genshi there were no problems, but
upon
upgrading to 0.4.1 I started getting a rather strange error
at a line
in one of those functions.  I then tried upgrading to 0.4.3,
but the
problem persisted.

The line in question was a string concatenation with +=,
like s += "foo"
Upon much experimenting I found that everything worked fine
if I
changed my string concatenations to the form s = s +
"foo"

Further experimentation proved that += doesn't work for ints
either.
Neither do -=, *=, etc.

Here's an example of what happens:

from genshi.template import MarkupTemplate
>>> templ = MarkupTemplate(
... """<?python
...     def foo():
...         i = 0
...         i = i + 1
...         i += 1
...         return i
...    ?>
...    <p>${foo()}</p>
... """)
>>> templ.generate().render()
Traceback (most recent call last):
  File "<stdin>", line 1, in ?
  File
"usr/local/lib/python2.4/site-packages/genshi/core.py&q
uot;, line
154, in render
    return encode(generator, method=method,
encoding=encoding)
  File
"usr/local/lib/python2.4/site-packages/genshi/output.py
", line
45, in encode
    output = u''.join(list(iterator))
  File
"usr/local/lib/python2.4/site-packages/genshi/output.py
", line
172, in __call__
    for kind, data, pos in stream:
  File
"usr/local/lib/python2.4/site-packages/genshi/output.py
", line
515, in __call__
    for kind, data, pos in stream:
  File
"usr/local/lib/python2.4/site-packages/genshi/output.py
", line
675, in __call__
    for kind, data, pos in chain(stream, [(None, None,
None)]):
  File
"usr/local/lib/python2.4/site-packages/genshi/output.py
", line
455, in __call__
    for ev in stream:
  File
"usr/local/lib/python2.4/site-packages/genshi/core.py&q
uot;, line
212, in _ensure
    for event in stream:
  File
"usr/local/lib/python2.4/site-packages/genshi/template/
markup.py",
line 291, in _match
    for event in stream:
  File
"usr/local/lib/python2.4/site-packages/genshi/template/
markup.py",
line 236, in _exec
    for event in stream:
  File
"usr/local/lib/python2.4/site-packages/genshi/template/
base.py",
line 420, in _eval
    result = data.evaluate(ctxt)
  File
"usr/local/lib/python2.4/site-packages/genshi/template/
eval.py",
line 136, in evaluate
    return eval(self.code, _globals, {'data': data})
  File "<string>", line 8, in <Expression
u'foo()'>
  File "<string>", line 4, in foo
NameError: global name 'UndefinedError' is not defined

In Genshi 0.4 the output of the same code is simply:
'<p>2</p>'
as I expected.

Is there something blatantly wrong that I'm doing here?  Is
there some
reason that we can't define and operate on local variables
in code
blocks?  I don't see why not...  and again, if I just change
i += 1 to
i = i + 1 there are no problems.

I've looked through the code a bit, but I can't figure out
what change
between 0.4 and 0.4.1 would cause this.

Erik

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Genshi" group.
To post to this group, send email to genshigooglegroups.com
To unsubscribe from this group, send email to
genshi-unsubscribegooglegroups.com
For more options, visit this group at http://gr
oups.google.com/group/genshi?hl=en
-~----------~----~----~----~------~----~------~--~---

Hi Erik,

Am 31.07.2007 um 22:39 schrieb Erik Bray:
> *whew* This one has had my stumped for a while.
>
> I have a Genshi template for Trac that I've been
working on that
> contains some rendering-related functions in a
<?python ?> code block.
>  Under version 0.4 of Genshi there were no problems,
but upon
> upgrading to 0.4.1 I started getting a rather strange
error at a line
> in one of those functions.  I then tried upgrading to
0.4.3, but the
> problem persisted.
>
> The line in question was a string concatenation with
+=, like s +=  
> "foo"
> Upon much experimenting I found that everything worked
fine if I
> changed my string concatenations to the form s = s +
"foo"
[snip]
> I've looked through the code a bit, but I can't figure
out what change
> between 0.4 and 0.4.1 would cause this.

Thanks for the detailed report, I've looked into the issue
and it  
should be fixed now, both on trunk and on the 0.4.x branch:

   <http://g
enshi.edgewall.org/changeset/693>

Please give it a try.

Thanks,
Chris
--
Christopher Lenz
   cmlenz at gmx.de
   http://www.cmlenz.net/


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Genshi" group.
To post to this group, send email to genshigooglegroups.com
To unsubscribe from this group, send email to
genshi-unsubscribegooglegroups.com
For more options, visit this group at http://gr
oups.google.com/group/genshi?hl=en
-~----------~----~----~----~------~----~------~--~---

Chris,
Thanks a lot for the quick fix.  It did the trick
perfectly.
It was also very educational about Genshi's internals for me
to try to
understand exactly what you fixed.

Erik

On 7/31/07, Christopher Lenz <cmlenzgmx.de> wrote:
>
> Hi Erik,
>
> Am 31.07.2007 um 22:39 schrieb Erik Bray:
> > *whew* This one has had my stumped for a while.
> >
> > I have a Genshi template for Trac that I've been
working on that
> > contains some rendering-related functions in a
<?python ?> code block.
> >  Under version 0.4 of Genshi there were no
problems, but upon
> > upgrading to 0.4.1 I started getting a rather
strange error at a line
> > in one of those functions.  I then tried upgrading
to 0.4.3, but the
> > problem persisted.
> >
> > The line in question was a string concatenation
with +=, like s +=
> > "foo"
> > Upon much experimenting I found that everything
worked fine if I
> > changed my string concatenations to the form s = s
+ "foo"
> [snip]
> > I've looked through the code a bit, but I can't
figure out what change
> > between 0.4 and 0.4.1 would cause this.
>
> Thanks for the detailed report, I've looked into the
issue and it
> should be fixed now, both on trunk and on the 0.4.x
branch:
>
>    <http://g
enshi.edgewall.org/changeset/693>
>
> Please give it a try.
>
> Thanks,
> Chris
> --
> Christopher Lenz
>    cmlenz at gmx.de
>    http://www.cmlenz.net/
>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Genshi" group.
To post to this group, send email to genshigooglegroups.com
To unsubscribe from this group, send email to
genshi-unsubscribegooglegroups.com
For more options, visit this group at http://gr
oups.google.com/group/genshi?hl=en
-~----------~----~----~----~------~----~------~--~---

Hi again,
Actually, I've found another issue.  In the same function
mentioned in
my original post, there is also a for loop that does
augmented assigns
to a variable defined in the body of that function.  Even
after
applying the change in r693, this causes and UndefinedError
to be
raised, because in this case self.locals[-1] refers to an
empty set
that was appended to self.locals by
TemplateASTTransformer.visitFor().

Should the scope for the for loop not be extended by its
containing
block?  My fix, which follows, seems to give the expected
results:

Index: genshi/template/eval.py
============================================================
=======
--- genshi/template/eval.py     (revision 695)
+++ genshi/template/eval.py     (working copy)
 -677,7
+677,7 
             self.locals.pop()

     def visitFor(self, node):
-        self.locals.append(set())
+        self.locals.append(set(self.locals[-1]))
         try:
             return ASTTransformer.visitFor(self, node)
         finally:


This problem may apply to other methods of
TemplateASTTransformer, but
I haven't tested that theory yet 

Thanks,
Erik

On 7/31/07, Christopher Lenz <cmlenzgmx.de> wrote:
>
> Hi Erik,
>
> Am 31.07.2007 um 22:39 schrieb Erik Bray:
> > *whew* This one has had my stumped for a while.
> >
> > I have a Genshi template for Trac that I've been
working on that
> > contains some rendering-related functions in a
<?python ?> code block.
> >  Under version 0.4 of Genshi there were no
problems, but upon
> > upgrading to 0.4.1 I started getting a rather
strange error at a line
> > in one of those functions.  I then tried upgrading
to 0.4.3, but the
> > problem persisted.
> >
> > The line in question was a string concatenation
with +=, like s +=
> > "foo"
> > Upon much experimenting I found that everything
worked fine if I
> > changed my string concatenations to the form s = s
+ "foo"
> [snip]
> > I've looked through the code a bit, but I can't
figure out what change
> > between 0.4 and 0.4.1 would cause this.
>
> Thanks for the detailed report, I've looked into the
issue and it
> should be fixed now, both on trunk and on the 0.4.x
branch:
>
>    <http://g
enshi.edgewall.org/changeset/693>
>
> Please give it a try.
>
> Thanks,
> Chris
> --
> Christopher Lenz
>    cmlenz at gmx.de
>    http://www.cmlenz.net/
>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Genshi" group.
To post to this group, send email to genshigooglegroups.com
To unsubscribe from this group, send email to
genshi-unsubscribegooglegroups.com
For more options, visit this group at http://gr
oups.google.com/group/genshi?hl=en
-~----------~----~----~----~------~----~------~--~---

Hey Erik,

Am 01.08.2007 um 17:21 schrieb Erik Bray:
> Actually, I've found another issue.  In the same
function mentioned in
> my original post, there is also a for loop that does
augmented assigns
> to a variable defined in the body of that function. 
Even after
> applying the change in r693, this causes and
UndefinedError to be
> raised, because in this case self.locals[-1] refers to
an empty set
> that was appended to self.locals by
TemplateASTTransformer.visitFor().
>
> Should the scope for the for loop not be extended by
its containing
> block?  My fix, which follows, seems to give the
expected results:

Ah, no, the problem was actually a stupid typo in my
previous fix :-P

The flatten() call was intended to make the name check look
at all  
the sets in the locals list. Of course, if you call flatten

(self.locals[-1]) instead of the intended
flatten(self.locals),  
you're still only looking at the top-most frame :-P

This should be fixed in trunk and 0.4.x now. Thanks for
testing the  
change and reporting this problem!

Cheers,
Chris
--
Christopher Lenz
   cmlenz at gmx.de
   http://www.cmlenz.net/


--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Genshi" group.
To post to this group, send email to genshigooglegroups.com
To unsubscribe from this group, send email to
genshi-unsubscribegooglegroups.com
For more options, visit this group at http://gr
oups.google.com/group/genshi?hl=en
-~----------~----~----~----~------~----~------~--~---

Aha!  That makes much more sense.  I knew something seemed
off about
copying the entire set.

Thanks,
Erik

On 8/1/07, Christopher Lenz <cmlenzgmx.de> wrote:
>
> Hey Erik,
>
> Am 01.08.2007 um 17:21 schrieb Erik Bray:
> > Actually, I've found another issue.  In the same
function mentioned in
> > my original post, there is also a for loop that
does augmented assigns
> > to a variable defined in the body of that
function.  Even after
> > applying the change in r693, this causes and
UndefinedError to be
> > raised, because in this case self.locals[-1]
refers to an empty set
> > that was appended to self.locals by
TemplateASTTransformer.visitFor().
> >
> > Should the scope for the for loop not be extended
by its containing
> > block?  My fix, which follows, seems to give the
expected results:
>
> Ah, no, the problem was actually a stupid typo in my
previous fix :-P
>
> The flatten() call was intended to make the name check
look at all
> the sets in the locals list. Of course, if you call
flatten
> (self.locals[-1]) instead of the intended
flatten(self.locals),
> you're still only looking at the top-most frame :-P
>
> This should be fixed in trunk and 0.4.x now. Thanks for
testing the
> change and reporting this problem!
>
> Cheers,
> Chris
> --
> Christopher Lenz
>    cmlenz at gmx.de
>    http://www.cmlenz.net/
>
>
> >
>

--~--~---------~--~----~------------~-------~--~----~
You received this message because you are subscribed to the
Google Groups "Genshi" group.
To post to this group, send email to genshigooglegroups.com
To unsubscribe from this group, send email to
genshi-unsubscribegooglegroups.com
For more options, visit this group at http://gr
oups.google.com/group/genshi?hl=en
-~----------~----~----~----~------~----~------~--~---