Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

bpo-32912: Upgrade warning for invalid escape sequences from silent to non-silent #5849

Closed
wants to merge 1 commit into from

Conversation

@Vgr255
Copy link

commented Feb 24, 2018

What the title says. Properly tested and ready for 3.8!

https://bugs.python.org/issue32912

:EDIT:

I don't know how to add a NEWS entry, but I have to go for now. I'll look into that tomorrow.

c->c_filename, LINENO(n),
NULL, NULL) < 0)
{
if (PyErr_ExceptionMatches(PyExc_DeprecationWarning)) {
if (PyErr_ExceptionMatches(PyExc_SyntaxWarning)) {

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Feb 24, 2018

Member

See the comment below.

This comment has been minimized.

Copy link
@Vgr255

Vgr255 Feb 24, 2018

Author

Yes, the end goal here is to get a SyntaxError, however this is not yet possible as we have to wait for two releases before this can happen, so we can't yet convert it to a SyntaxError. I probably should update that comment, though (I'll do that tomorrow when I get home).

@@ -6109,7 +6109,7 @@ PyUnicode_DecodeUnicodeEscape(const char *s,
if (result == NULL)
return NULL;
if (first_invalid_escape != NULL) {
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
if (PyErr_WarnFormat(PyExc_SyntaxWarning, 1,

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Feb 24, 2018

Member

This is not related to syntax.

This comment has been minimized.

Copy link
@Vgr255

Vgr255 Feb 24, 2018

Author

This warning was added back in 3.6 when we started deprecating the invalid escapes. In 3.8, we're upgrading these to SyntaxWarning so that they're no longer silent by default, and it will eventually be a SyntaxError. The end goal here is to raise a SyntaxError for this use, but this will have to wait for two more releases.

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Feb 24, 2018

Member

This warning is emitted by the "unicode-escape" codec.

>>> b'\\z'.decode('unicode-escape')
__main__:1: DeprecationWarning: invalid escape sequence '\z'
'\\z'

This comment has been minimized.

Copy link
@Vgr255

Vgr255 Feb 24, 2018

Author

Yes, and this warning was added as part of bpo-27364 to keep consistency with the rest of the language. It only makes sense to upgrade this warning along with everything else, or else this codec falls out of sync with the rest of the language.

@@ -1255,7 +1255,7 @@ PyObject *PyBytes_DecodeEscape(const char *s,
if (result == NULL)
return NULL;
if (first_invalid_escape != NULL) {
if (PyErr_WarnFormat(PyExc_DeprecationWarning, 1,
if (PyErr_WarnFormat(PyExc_SyntaxWarning, 1,

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Feb 24, 2018

Member

This is not related to syntax.

This comment has been minimized.

Copy link
@Vgr255

Vgr255 Feb 24, 2018

Author

This warning was added back in 3.6 when we started deprecating the invalid escapes. In 3.8, we're upgrading these to SyntaxWarning so that they're no longer silent by default, and it will eventually be a SyntaxError. The end goal here is to raise a SyntaxError for this use, but this will have to wait for two more releases.

This comment has been minimized.

Copy link
@serhiy-storchaka

serhiy-storchaka Feb 24, 2018

Member

No, this warning is emitted by codecs.escape_decode() (which is used in pickle).

>>> codecs.escape_decode(b'\\z')
__main__:1: DeprecationWarning: invalid escape sequence '\z'
(b'\\z', 2)

This comment has been minimized.

Copy link
@Vgr255

Vgr255 Feb 24, 2018

Author

Yes, and this warning was added as part of bpo-27364 to keep consistency with the rest of the language. It only makes sense to upgrade this warning along with everything else, or else the codecs module falls out of sync with the rest of the language.

@serhiy-storchaka
Copy link
Member

left a comment

The codecs module shouldn't raise SyntaxWarning or SyntaxError. It should raise DeprecationWarning, which later will be converted to UnicodeDecodeError.

@Vgr255

This comment has been minimized.

Copy link
Author

commented Mar 27, 2018

I haven't had the time to work on this, sorry! I'll get back to it hopefully sometime soon.

@tirkarthi

This comment has been minimized.

Copy link
Contributor

commented May 16, 2019

I think this was done with #9652 and the bpo issue is also marked as resolved.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.