KEMBAR78
Don't crash on unexpected PY2 except syntax by gvanrossum · Pull Request #4011 · python/mypy · GitHub
Skip to content

Conversation

gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Sep 26, 2017

Fixes #3772

Still needs a test.

raise RuntimeError("'{}' has non-Name name.".format(ast27.dump(item)))

vs = [produce_name(h) for h in handlers]
self.fail("Sorry, `except <expr>, (<tuple>)` is not supported", item.lineno, item.col_offset)
Copy link
Member

Choose a reason for hiding this comment

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

This could be something other than a tuple however. For example, this works fine at runtime:

try:
    raise Exception('foo', 'bar')
except Exception, [a, b]:
    print a, b  # prints foo bar

Perhaps just "Unsupported exception handler syntax ...". If you want a specific warning, you can check that the item.name is an ast27.Tuple or ast27.List.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll change it to except <expr>, <anything but a name> -- it's syntactically an assignment target. I still don't think it's worth trying to support other syntax -- we just need to not crash.

Copy link
Member

@emmatyping emmatyping left a comment

Choose a reason for hiding this comment

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

Looks good 👍

@gvanrossum
Copy link
Member Author

Whoa, needs tests first.

@emmatyping
Copy link
Member

Sorry, yes it needs tests. I meant the changes look good (beyond needing tests).

@emmatyping
Copy link
Member

Travis flaked on 3.7, so I restarted it.

@gvanrossum gvanrossum merged commit 5252e82 into python:master Sep 26, 2017
@gvanrossum gvanrossum deleted the fix-except-tuple branch September 26, 2017 20:55
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.

2 participants