Skip to content

Conversation

@stephan-hof
Copy link
Member

This ports Eval.py to python3 by using the methods from compile.py.

As a side effect I brought back support for used_names in RestrictingNodeTransformer.
I know nowadays used_names could have been a set(), but I didn't dare to change the API.
Since used_names is used by other code outside of RestrictedPython.

Copy link
Member

@loechel loechel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@loechel
Copy link
Member

loechel commented Mar 4, 2017

@stephan-hof could you please rebase and merge afterwards

This gets rid of the dependency to know how the bytecode looks like.
I guess this particular code was used at the early days of restricted
python. To see the speed penaltiy. However I cannot see how this can be
utilized by other libraries.
* Using `keys` is not modern anymore.
* Using `has_key` is not modern anymore.
* Proper variable names
* Comments in the same line is not very pep8.
@stephan-hof stephan-hof merged commit 14ee5ed into Python3_update Mar 6, 2017
@stephan-hof stephan-hof deleted the port-eval-py branch March 6, 2017 07:51
Copy link
Member

@icemac icemac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the question I had.

if self.ucode is None:
# Use the standard compiler.
co = compile(self.expr, '<string>', 'eval')
exp_node = compile(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is prepUnrestrictedCode actually still needed?
I think getting SyntaxErrors and used names can be done by compile_restricted_eval.
Or do I miss something here?
(Maybe this can be delayed after the release to stay backwards compatible to see if some code uses this method or the ucode attribute.)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right, the only justification for this method is the ucode attribute.
I did a quick search and found one user of it: https://github.com/zopefoundation/DocumentTemplate/blob/6b526d4b76f5496de5049f365a09ab712751c532/src/DocumentTemplate/DT_Util.py#L212

My suggestion would be to keep this method and ucode. To stay backwards compatible.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okay.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants