diff --git a/Doc/whatsnew/3.8.rst b/Doc/whatsnew/3.8.rst index bf8d8f15434..0360be604f5 100644 --- a/Doc/whatsnew/3.8.rst +++ b/Doc/whatsnew/3.8.rst @@ -442,6 +442,13 @@ Changes in Python behavior in the leftmost :keyword:`!for` clause). (Contributed by Serhiy Storchaka in :issue:`10544`.) +* The compiler now produces a :exc:`SyntaxWarning` when identity checks + (``is`` and ``is not``) are used with certain types of literals + (e.g. strings, ints). These can often work by accident in CPython, + but are not guaranteed by the language spec. The warning advises users + to use equality tests (``==`` and ``!=``) instead. + (Contributed by Serhiy Storchaka in :issue:`34850`.) + Changes in the Python API ------------------------- diff --git a/Lib/test/test_ast.py b/Lib/test/test_ast.py index 2c8d8ab7e3f..897e705a42c 100644 --- a/Lib/test/test_ast.py +++ b/Lib/test/test_ast.py @@ -1146,11 +1146,12 @@ class ASTValidatorTests(unittest.TestCase): tests = [fn for fn in os.listdir(stdlib) if fn.endswith(".py")] tests.extend(["test/test_grammar.py", "test/test_unpack_ex.py"]) for module in tests: - fn = os.path.join(stdlib, module) - with open(fn, "r", encoding="utf-8") as fp: - source = fp.read() - mod = ast.parse(source, fn) - compile(mod, fn, "exec") + with self.subTest(module): + fn = os.path.join(stdlib, module) + with open(fn, "r", encoding="utf-8") as fp: + source = fp.read() + mod = ast.parse(source, fn) + compile(mod, fn, "exec") class ConstantTests(unittest.TestCase): diff --git a/Lib/test/test_grammar.py b/Lib/test/test_grammar.py index 3d8b1514f0c..3ed19ff1cb0 100644 --- a/Lib/test/test_grammar.py +++ b/Lib/test/test_grammar.py @@ -1226,11 +1226,33 @@ class GrammarTests(unittest.TestCase): if 1 > 1: pass if 1 <= 1: pass if 1 >= 1: pass - if 1 is 1: pass - if 1 is not 1: pass + if x is x: pass + if x is not x: pass if 1 in (): pass if 1 not in (): pass - if 1 < 1 > 1 == 1 >= 1 <= 1 != 1 in 1 not in 1 is 1 is not 1: pass + if 1 < 1 > 1 == 1 >= 1 <= 1 != 1 in 1 not in x is x is not x: pass + + def test_comparison_is_literal(self): + def check(test, msg='"is" with a literal'): + with self.assertWarnsRegex(SyntaxWarning, msg): + compile(test, '', 'exec') + with warnings.catch_warnings(): + warnings.filterwarnings('error', category=SyntaxWarning) + with self.assertRaisesRegex(SyntaxError, msg): + compile(test, '', 'exec') + + check('x is 1') + check('x is "thing"') + check('1 is x') + check('x is y is 1') + check('x is not 1', '"is not" with a literal') + + with warnings.catch_warnings(): + warnings.filterwarnings('error', category=SyntaxWarning) + compile('x is None', '', 'exec') + compile('x is False', '', 'exec') + compile('x is True', '', 'exec') + compile('x is ...', '', 'exec') def test_binary_mask_ops(self): x = 1 & 1 @@ -1520,9 +1542,11 @@ class GrammarTests(unittest.TestCase): self.assertEqual(16 // (4 // 2), 8) self.assertEqual((16 // 4) // 2, 2) self.assertEqual(16 // 4 // 2, 2) - self.assertTrue(False is (2 is 3)) - self.assertFalse((False is 2) is 3) - self.assertFalse(False is 2 is 3) + x = 2 + y = 3 + self.assertTrue(False is (x is y)) + self.assertFalse((False is x) is y) + self.assertFalse(False is x is y) def test_matrix_mul(self): # This is not intended to be a comprehensive test, rather just to be few diff --git a/Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst b/Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst new file mode 100644 index 00000000000..bc5d5d1d580 --- /dev/null +++ b/Misc/NEWS.d/next/Core and Builtins/2018-09-30-11-19-55.bpo-34850.CbgDwb.rst @@ -0,0 +1,5 @@ +The compiler now produces a :exc:`SyntaxWarning` when identity checks +(``is`` and ``is not``) are used with certain types of literals +(e.g. strings, ints). These can often work by accident in CPython, +but are not guaranteed by the language spec. The warning advises users +to use equality tests (``==`` and ``!=``) instead. diff --git a/Python/compile.c b/Python/compile.c index 45e78cb22cd..5aebda0da4d 100644 --- a/Python/compile.c +++ b/Python/compile.c @@ -2284,6 +2284,47 @@ compiler_class(struct compiler *c, stmt_ty s) return 1; } +/* Return 0 if the expression is a constant value except named singletons. + Return 1 otherwise. */ +static int +check_is_arg(expr_ty e) +{ + if (e->kind != Constant_kind) { + return 1; + } + PyObject *value = e->v.Constant.value; + return (value == Py_None + || value == Py_False + || value == Py_True + || value == Py_Ellipsis); +} + +/* Check operands of identity chacks ("is" and "is not"). + Emit a warning if any operand is a constant except named singletons. + Return 0 on error. + */ +static int +check_compare(struct compiler *c, expr_ty e) +{ + Py_ssize_t i, n; + int left = check_is_arg(e->v.Compare.left); + n = asdl_seq_LEN(e->v.Compare.ops); + for (i = 0; i < n; i++) { + cmpop_ty op = (cmpop_ty)asdl_seq_GET(e->v.Compare.ops, i); + int right = check_is_arg((expr_ty)asdl_seq_GET(e->v.Compare.comparators, i)); + if (op == Is || op == IsNot) { + if (!right || !left) { + const char *msg = (op == Is) + ? "\"is\" with a literal. Did you mean \"==\"?" + : "\"is not\" with a literal. Did you mean \"!=\"?"; + return compiler_warn(c, msg); + } + } + left = right; + } + return 1; +} + static int cmpop(cmpop_ty op) { @@ -2363,6 +2404,9 @@ compiler_jump_if(struct compiler *c, expr_ty e, basicblock *next, int cond) return 1; } case Compare_kind: { + if (!check_compare(c, e)) { + return 0; + } Py_ssize_t i, n = asdl_seq_LEN(e->v.Compare.ops) - 1; if (n > 0) { basicblock *cleanup = compiler_new_block(c); @@ -3670,6 +3714,9 @@ compiler_compare(struct compiler *c, expr_ty e) { Py_ssize_t i, n; + if (!check_compare(c, e)) { + return 0; + } VISIT(c, expr, e->v.Compare.left); assert(asdl_seq_LEN(e->v.Compare.ops) > 0); n = asdl_seq_LEN(e->v.Compare.ops) - 1;