homepage

Seems there is a bug in the implementation of the IMPORT_NAME opcode in ceval.c.
If the level argument is -1, it is not passed to __import__ (should never happen, but looks correct). If it is an integer != -1 in C long range, it is passed to __import__ (this is correct). But if it is not integer (e.g. None) or can't be converted to C long, an exception is set and __import__ is called with level and not cleared error (this is wrong).
In correct bytecode the level argument can be either integer or None. Default __import__ accepts only integers as the level argument and checks the range. Proposed patch makes the code always passing the level argument to __import__ unless it is None.

My suspicion is the -1 aspect is a hold-over from that being the default level value in Python 2.7 that no one removed.
The patch LGTM. Do we need a test for this for some reason?

Thak you for the explanation Brett.
Since current "import" without "from" is not broken, I suppose that the raised exception is cleared somewhere later. The patch doesn't have visible effect (unless may be using nonstandard loaders or __import__, don't know). It just cleans up the code, avoid raising unneeded exception (small performance boost) and may be fix some hidden non-reproducible bug.

Then I say go ahead and commit it.

Sorry, I was wrong. The level argument is always non-negative integer (if a bytecode is not corrupted). The second branch can be just removed.
Following patch also fixes the validation of the ImportFrom AST node.

LGTM

New changeset e3164c9edb0b by Serhiy Storchaka in branch 'default':
Issue #27352: Correct the validation of the ImportFrom AST node and simplify
https://hg.python.org/cpython/rev/e3164c9edb0b 

Thanks Brett.

You broke Python! Example:
http://buildbot.python.org/all/builders/AMD64%20Snow%20Leop%203.x/builds/4853/steps/test/logs/stdio
======================================================================
FAIL: test_importfrom (test.test_ast.ASTValidatorTests)
----------------------------------------------------------------------
Traceback (most recent call last):
 File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ast.py", line 757, in test_importfrom
 self.stmt(imp, "level less than -1")
 File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ast.py", line 579, in stmt
 self.mod(mod, msg)
 File "/Users/buildbot/buildarea/3.x.murray-snowleopard/build/Lib/test/test_ast.py", line 571, in mod
 self.assertIn(msg, str(cm.exception))
AssertionError: 'level less than -1' not found in 'Negative ImportFrom level'

New changeset e5063a82f490 by Serhiy Storchaka in branch 'default':
Issue #27352: Fixed an error message in a test.
https://hg.python.org/cpython/rev/e5063a82f490 

Thanks Victor!