classification
Title: Improve the warning message for invalid escape sequences
Type: enhancement Stage: resolved
Components: Interpreter Core Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: eric.smith Nosy List: Tim.Graham, abarry, eric.smith, ezio.melotti, martin.panter, ncoghlan, ned.deily, petr.viktorin, python-dev, r.david.murray, rhettinger, serhiy.storchaka, yan12125, yselivanov
Priority: release blocker Keywords: patch

Created on 2016-09-13 15:10 by yan12125, last changed 2017-03-31 16:36 by dstufft. This issue is now closed.

Files
File name Uploaded Description Edit
28128.diff eric.smith, 2016-09-16 14:22 review
28128-2.diff eric.smith, 2016-10-31 00:58
28128-3.diff eric.smith, 2016-10-31 01:17
28128-3.diff serhiy.storchaka, 2016-10-31 08:43 Regenerated for review review
28128-4.diff serhiy.storchaka, 2016-10-31 09:54 review
28128-5.diff serhiy.storchaka, 2016-10-31 10:39 review
28128-6.diff serhiy.storchaka, 2016-10-31 12:02 review
28128-7.diff serhiy.storchaka, 2016-10-31 12:36 review
Pull Requests
URL Status Linked Edit
PR 552 closed dstufft, 2017-03-31 16:36
Messages (55)
msg276285 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-09-13 15:10
In issue27364, invalid escape sequences in string literals are deprecated. Currently the deprecation message is not so useful when fixing lots of files in one or more large projects. For example, I have two files foo.py and bar.py:

# foo.py
import bar

# bar.py
print('\d')

It gives:
$ python3.6 -W error foo.py
Traceback (most recent call last):
  File "foo.py", line 1, in <module>
    import bar
DeprecationWarning: invalid escape sequence '\d'

My idea is that the warning message can be improved to provide more information. In http://bugs.python.org/issue27364#msg269373 it's proposed to let a linter check such misuses. It's useful within a single project. For a project that depends on lots of external projects, a linter is not enough. Things are worse when __import__, imp or importlib are involved, or sys.path is modified. I have to either add some codes or use a debugger to show which module is imported.

For above reasons, I propose to add at least the filename and the line number to the warning message. For example:

$ ./python -W error foo.py
Traceback (most recent call last):
  File "foo.py", line 1, in <module>
    import bar
  File "/home/yen/Projects/cpython/build/bar.py", line 1
    print('\d')
         ^
SyntaxError: (deprecated usage) invalid escape sequence '\d'

With that I can know which file or project I should blame.

Added some of reviewers from issue27364 to nosy list.
msg276286 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-09-13 15:11
And I'd like to ask Ned: for me it's an improvement of an existing feature, so I guess it can enter the 3.6 branch?
msg276293 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-09-13 15:43
It sounds like a fix but let's see the final patch first.  If a core developer wants to apply it to the default branch for 3.7, we can decide whether it should go into 3.6, too.
msg276326 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-09-13 19:04
The error can't be a SyntaxError, it must be a DeprecationWarning.  If you can improve the deprecation warning text, I'd be in favor of calling that a fix to the original feature and put it in 3.6.  The deprecation warning can even say that this will be a SyntaxError some time in the future.
msg276341 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-09-13 20:45
Hello, and thanks! I'll work on a patch this week, or at most next week. I will make it so that it's completely uncontroversial to apply it to 3.6 as well (won't change the actual feature, only prettify the error message), so no need to worry about that :)
msg276364 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2016-09-14 00:25
See also Issue 28028. Serhiy suggested translating warnings to SyntaxWarning in general. Looks like that may help narrowing down the location of escaping problems.
msg276548 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-09-15 10:51
Besides converting the DeprecationWarning to a Syntax{Error,Warning}, I don't see an easy way to include the offending line (or even file). The place in the code where the strings are created has no idea *where* they are being defined. AIUI, either we special-case this, or we resolve #28028 (but I don't think the latter can go in 3.6).
msg276560 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2016-09-15 14:53
Are SyntaxWarnings silent by default?  If not it can't even go into 3.7.
msg276641 - (view) Author: Tim Graham (Tim.Graham) * Date: 2016-09-15 23:43
I hope the message can be improved for Python 3.6 as the warnings I see when running Django's test suite are rather useless to help find and fix the issues:

cpython/Lib/importlib/_bootstrap.py:205: DeprecationWarning: invalid escape sequence '\:'

Grepping for these characters patterns to try to figure out where they come from is quite difficult.

I did track down one instance in a docstring:
https://github.com/django/django/blob/82f8996785751c413e3b4ac12bf387f781c200d8/django/contrib/gis/maps/google/__init__.py#L41

Is the correct resolution to add an r prefix to the string? It could be nice if the release notes contained some guidance on that.
msg276646 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-09-16 00:13
There definitely needs to be a better message for that. The problem is that the parser doesn't have access to the whole string (of course; it's being constructed!), so I think there are several possible venues here:

- Change DeprecationWarning to display the line which is faulty (unlikely);
- Promote these specific warnings to SyntaxErrors *only* if the warning is an error (i.e. Python was run with -W error), in which case the compilation of code is stopped either way, so there is no drawback (but only being a syntax error "sometimes" is likely to confuse people);
- Use a different warning class, but presumably we'd need a new one and we can't do that now since we're in beta (unless Ned thinks this is appropriate, but I doubt it).

Anything else, maybe? I'm sure there is a way to properly fix it; my personal favourite all things considered is the second option (which I've thought over and over recently), but I'm open to other, less intrusive alternatives.

My intention is to make this deprecation the least painful possible for application and third-party library developers. As such, I think adding a note to the docs (possibly in What's New, but probably also under a few other related places) is good, and probably doing a preemptive SO question+answer for people who stumble upon this.

I've added to nosy a couple of people whose feedback I believe could be very helpful.
msg276658 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-16 01:40
Adding a "DeprecatedSyntaxWarning" that's a subclass of both DeprecationWarning and SyntaxWarning (and hence silenced by default while still having the extra SyntaxWarning attributes) strikes me as a reasonable step to take here, even though we're into the beta period already - if we couldn't effectively respond to this kind of pre-release usability feedback, the beta period would be nowhere near as effective.

Ideally, this would also let you hook into the same system that ensures syntax warnings are correctly attributed to the code containing them from a warning filter perspective, rather than the import statement triggering the compilation. One possible way to do that would be to postpone generating this error to a later stage of the code generation pipeline rather than doing it directly in the parser.

Otherwise folks trying to constrain "-Werror" to just their own code and not their dependencies are going to have a bad time of it. If we find we can't readily fix that aspect of the change, then I'd go so far as proposing to revert it and trying again for 3.7. If we do find we need to take that step, we could still encourage linter developers to start complaining about unknown escapes during the 3.6 cycle.

P.S. As general background, the "No new features after beta 1" rule is more accurately written as "No new features after beta 1, *except* for features needed to resolve usability problems reported with changes that already landed in beta 1". The biggest one of those I've seen was the PEP 492 implementation for 3.5, where the implementation data model was originally still just "generators with a particular flag set" (similar to the 3.4 model), but the Tornado and Cython folks found that unworkable after the beta went out, so we made both the Python API and C API expose them as distinct classes with different attributes (cr_* instead of gi_*) and different ABCs that the asyncio checked for.
msg276661 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-09-16 02:01
Thank you Nick for the useful feedback! I think that a subclass of DeprecationWarning and SyntaxWarning would be a good idea; I'll play around with that.

As far as when the warning should occur, I agree that erroring out at the compile step isn't optimal, however (AFAIK) strings aren't checked again once they've been created; they're assumed to be correct (and checking them at runtime would presumably add a performance hit to *all* strings, which I think is too big a price to pay).

On the other hand, a way to ignore invalid escapes in certain files could be considered, but we need to be somewhat careful since some people might choose to silence all warnings, thus defeating the purpose.
msg276662 - (view) Author: Yury Selivanov (yselivanov) * (Python committer) Date: 2016-09-16 02:02
BTW, this will also help to make warnings more friendly in #26182.
msg276663 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-16 02:05
I realised I wasn't entirely clear about the "warning misattribution" problem that's implied by Chi Hsuan's problem report, so here's the behaviour when using "-W all" rather than "-W error":

    $ echo "print('\d')" > bad_escape.py
    $ echo "import bad_escape" > escape_warning.py
    $ ./python -W all escape_warning.py 
    _frozen_importlib:205: DeprecationWarning: invalid escape sequence '\d'
    \d

That's a misattribution - the warning should be reported against the module being imported, not against the import system itself.

Compare that to the attribution of the old SyntaxWarning for assignments prior to a scope declaration:

    $ cat syntax_warning.py 
    def f():
        x = 1
        global x
    $ python3 -c "import syntax_warning" 
    /home/ncoghlan/devel/py36/syntax_warning.py:3: SyntaxWarning: name 'x' is assigned to before global declaration
      global x

(Run with 3.5, as that became a full SyntaxError for 3.6)
msg276665 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-16 02:18
Regarding when I think the error should be generated, we definitely want the warning to be happening at compile time, but the "compile step" is actually a series of substeps.

The point where the string parser is processing string escapes is *not* the best place to complain about unrecognised ones, as it doesn't have the ability to report the appropriate context for where the offending string exists in the code base (hence this issue report, and the misattribution problem in the generated warning).

Instead, you probably want to look at delaying generation of the error until the point where AST Bytes and Str nodes are being generated for inclusion in the AST, as at that point the code generator has access to full file, line, and column information regarding the location of the problematic escape.

However, I also expect you'll run into a problem where you'll need to be able to embed *something* in the processed string that lets the latter stage of the pipeline detect that there was an unknown escape rather than a properly escaped usage of "\\" (which is presumably why the "immediate warning" approach was attempted first - it doesn't need that ability to communicate with the latter stage of the pipeline).
msg276666 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-09-16 02:26
Hmm, I see; I'll need to dig a bit deeper get to and understand that part of the compile process better. I'll look up where SyntaxErrors are generated (since they have access to at least the line number at that point), and try to hook it up from there.
msg276668 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-16 02:55
I've added Eric Smith to the nosy list as well, as he's been working on that part of the compilation pipeline for the f-string implementation.

Eric, the context here is that the new deprecation warning for unknown escapes is currently being misattributed to the code doing the compilation rather than to the code containing the offending (non-)escape sequence.

That's making it difficult for folks to act effectively on the warnings when they receive them, and also poses problems for correctly filtering out warnings coming from code in dependencies rather than an application's own code.
msg276698 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-09-16 11:35
Couldn't we create a private version of PyUnicode_DecodeUnicodeEscape, to be called by ast.c, which passes back invalid escape info? Then have the actual warning raised by ast.c, which knows enough about the context to generate a better error/warning. I think we'd only be able to report on he first error in a string, though, but I haven't thought it all the way through.

I believe we'd only need to modify decode_unicode_with_escapes() in ast.c, which is called for both regular strings and f-string. And of course PyUnicode_DecodeUnicodeEscape would have to call the new private version and do the right thing.
msg276715 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-09-16 14:22
Here is an extremely rough patch that shows the basic concept. I named the private function _PyUnicode_DecodeUnicodeEscape.

The problems with this patch are:
1. it always raises an error, not a warning
2. the private function isn't declared in a .h file
3. the name of the private function needs some thought
4. only the first invalid escape in a string is reported
5. I don't report the correct location in the string with the invalid escape
6. there may well be a memory leak
7. PEP 7 problems

#1 is because I was too lazy to refactor ast_error() to format the string I need without raising an error.

#5 could be solved with a callback and something to record multiple bad escapes per string, if we want to go that far. We'd have to decide how to show this. Multiple warnings, or one warning with multiple bad chars?

The rest of it is just quality of implementation stuff that we can work out if the approach is sound.
msg276716 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-09-16 14:23
I forgot: this is what Nick's example now looks like:

$ ./python -Wall escape_warning.py
Traceback (most recent call last):
  File "escape_warning.py", line 1, in <module>
    import bad_escape
  File "/home/eric/local/python/cpython/bad_escape.py", line 1
    print('\d')
         ^
SyntaxError: invalid escape sequence \d
msg276717 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-09-16 14:25
Also, I assume this is a problem with all such syntax warnings: you only see this warning/error when the file is originally compiled. Once the .pyc file exists, you'll never see a warning or error. Maybe that's okay, but it means there's a certain class of installations (such as .pyc compiled at install time) that won't be able to know these warnings exist.

This screwed me up for a while when I was developing this patch. The warnings disappeared!
msg276720 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-09-16 14:34
Personally I'd be fine with only one warning, reporting the first invalid escape. Presumably the string would be checked as a whole, or would get an r prefix. Patch seems like a good start; bytes would also need updating in that regard. Don't worry too much about the details, we can iron that out later :)

Also yeah, the fact that warnings are triggered at compile time results in surprising behaviour even for me. I'm liking the idea of a DeprecatedSyntaxWarning more.

I'll try to get some work done in that regard by the end of next week, but life's been somewhat busy so I can't give any guarantees. Thank you for your patch draft though, it helps me to figure out how I should tackle this!
msg276728 - (view) Author: Tim Graham (Tim.Graham) * Date: 2016-09-16 16:59
Eric, your patch was good enough to allow me to easily identify and fix all the warnings in Django: https://github.com/django/django/pull/7254. Thanks!
msg276730 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-09-16 17:46
The basic concept LGTM.

first_invalid_escape_char is redundant, it is just s[first_invalid_escape_idx]. Or maybe better to return a pointer instead of an index.

bytes literals need similar solution.
msg276731 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-09-16 18:04
Tim: Cool! That's way more useful than I thought it would be.

Serhiy: It's a proof of concept. Lots of design remains to be done. I'm not sure we've agreed on the concept yet, so I don't think it's worthwhile designing the API.
msg276785 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-09-17 12:06
Eric's basic approach sounds fine to me.

The "pre-compiled .pyc files won't trigger SyntaxWarning" problem isn't new, as it exists for the old 3.5 warnings as well (-B prevents writing bytecode, which may be handy while working on this. Unfortunately, there's no equivalent to prevent reading it except deleting the offending bytecode file):

  $ python3 -B -c "import syntax_warning" 
  /home/ncoghlan/devel/py36/syntax_warning.py:3: SyntaxWarning: name 'x' is assigned to before global declaration
    global x
  $ python3 -c "import syntax_warning" 
  /home/ncoghlan/devel/py36/syntax_warning.py:3: SyntaxWarning: name 'x' is assigned to before global declaration
    global x
  $ python3 -c "import syntax_warning" 
  $ rm __pycache__/syntax_warning.cpython-35.pyc 
  $ python3 -B -c "import syntax_warning" 
  /home/ncoghlan/devel/py36/syntax_warning.py:3: SyntaxWarning: name 'x' is assigned to before global declaration
    global x

As long as folks are running their tests at least once in fresh environments they'll see the warning.
msg277916 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-10-02 23:48
Ping. I'd like for this to get merged in beta 2; should I (or Eric if he wants to) get to work on this? Is the DeprecatedSyntaxWarning subclass route still desired?
msg277967 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-03 17:13
Eric's basic approach sounds fine to me, as it gets the traceback in the right place (i.e. blaming the code being compiled, not the code doing the import).

For beta 2, how about we just go with a plain SyntaxWarning? Since users running pre-compiled modules won't see it regardless, so it will be effectively silenced by default for end users of pre-built Python applications.
msg278251 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-07 16:00
Sorry for not responding earlier. It's unlikely I'll have time for this before beta 2, although I can probably get to it after that and before beta 3. Don't let me stop someone else from improving on the patch.
msg279650 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-10-29 03:46
Following Ned's email to Python-Dev, I'm upping the priority of this issue to "deferred blocker".

I haven't had a lot of time to work on this, but I had time to think about it, and I think that resolving #28028 would be the best way to solved this. I pinged this issue as well and added several people to nosy there. I've also added it as a dependency of this issue.

To be clear, I really like Eric's approach to this, but I think that resolving #28028 would let us kill a couple of issues with the same patch. My priority is getting this issue here resolved, though, so either way works. I doubt I'll have much time to actually work on this; life's been busy and Python isn't my priority right now (as much as I'd like it to be).
msg279654 - (view) Author: Nick Coghlan (ncoghlan) * (Python committer) Date: 2016-10-29 05:10
Petr, is there any chance you or someone from your team could take a look at this and the other issue Emanuel referenced? These warnings are likely to pose a usability problem when upgrading Python in Fedora in their current state, so it would be good to see them addressed prior to the 3.6.0 release.
msg279724 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-30 14:57
I'll work on this later today.
msg279754 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 00:58
Here's an updated patch, that fixes some problems with the earlier patch, and adds equivalent support for bytes.

HOWEVER, I can't get the warnings machinery to raise a DeprecationWarning that would have all of the equivalent information that an actual SyntaxError would have. So this patch still raises SyntaxError, but with a better error context.

I'm going to keep plugging away at this, but I'm hoping that someone with more experience with warnings using the C-API can chime in with some advice. Given the tight deadline, I can use all of the help I can get.

The two functions that need help are decode_bytes_with_escapes and decode_unicode_with_escapes in ast.c.
msg279757 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 01:17
Oops, use 28128-3.diff.
msg279759 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-10-31 01:59
Thank you Eric. Have you looked at making a new DeprecatedSyntaxWarning subclass of both DeprecationWarning and SyntaxWarning? Hopefully that's of some help.

I don't see a review link, but from a quick glance this looks good. Thanks :)
msg279771 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 08:36
I'll take a look at it, Emanuel. But I can't promise how much progress I'll be able to make today. I also think that at that point it becomes so complex that it fails Ned's test for inclusion in 3.6.
msg279776 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-31 09:54
Updated patch emits DeprecationWarning instead of SyntaxError. I still not tested it.
msg279778 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-31 10:39
Fixed bytes literals decoding (test_codecs was failed).
msg279781 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 11:57
Serihy:

I had tried this approach earlier, but it doesn't work. With your -5.diff patch, the output is (using Nick's test case):

$ rm -rf __pycache__/ ; ./python -Werror escape_warning.py 
Traceback (most recent call last):
  File "escape_warning.py", line 1, in <module>
    import bad_escape
DeprecationWarning: invalid escape sequence \d
$ 

With my -4.diff patch, you get the desired full stack trace:

$ rm -rf __pycache__/ ; ./python -Wall escape_warning.py 
Traceback (most recent call last):
  File "escape_warning.py", line 1, in <module>
    import bad_escape
  File "/home/eric/local/python/cpython/bad_escape.py", line 1
    print('\d')
         ^
SyntaxError: invalid escape sequence \d
$ 

The trick is: how to make the DeprecationWarning version produce output similar to the SyntaxError case? Note that with DeprecationWarning, you don't see the line in bad_escape.py that actually contains the string with the invalid escape.
msg279782 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-31 12:02
Added additional tests.
msg279783 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 12:06
Also, you'll note that with or without your patch, you get the same behavior. The code in hg already raises DeprecationWarning, just in a different place. So unless we can improve the DeprecationWarning output, we're better off doing nothing.
msg279784 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-31 12:36
Following patch just raises SyntaxError if DeprecationWarning was raised as error. Still needed tests for this.

> Also, you'll note that with or without your patch, you get the same behavior.

Not the same. New warnings contain correct information about a file and a line.

$ ./python-unpatched -Wa escape_warning.py
_frozen_importlib:205: DeprecationWarning: invalid escape sequence '\d'
\d

$ ./python-patched -Wa escape_warning.py
/home/serhiy/py/cpython-3.6/bad_escape.py:2: DeprecationWarning: invalid escape sequence \d
  print('\d')
\d

$ ./python-unpatched -We escape_warning.py
Traceback (most recent call last):
  File "escape_warning.py", line 1, in <module>
    import bad_escape
DeprecationWarning: invalid escape sequence '\d'

$ ./python-patched -We escape_warning.py
Traceback (most recent call last):
  File "escape_warning.py", line 1, in <module>
    import bad_escape
  File "/home/serhiy/py/cpython-3.6/bad_escape.py", line 2
    print('\d')
         ^
SyntaxError: invalid escape sequence \d
msg279785 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 12:42
I'm not in front of a computer at the moment, but the output looks good. Also, my very quick glance at -7.diff's warn_invalid_escape_sequence looks reasonable, although I can't say for sure whether raising the error in PyErr_WarnExplicitObject followed by raising another error in ast_error is absolutely correct (it's outside my expertise).
msg279789 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-31 13:22
New changeset 259745f9a1e4 by Eric V. Smith in branch 'default':
Issue 28128: Print out better error/warning messages for invalid string escapes.
https://hg.python.org/cpython/rev/259745f9a1e4
msg279790 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 13:23
I've pushed this to the default branch. I'll watch the buildbots.

Then Ned can decide if this goes in to 3.6.
msg279791 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-10-31 13:33
Searching on GitHub it seems to me that the most frequent issue with supporting Python 3.6 is eliminating or silencing warnings about invalid escape sequences. Any help with this is very important.
msg279793 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-10-31 15:00
As Nick pointed out in an earlier message on this thread and as Serhiy observed on GitHub issues, backporting this patch to 3.6 is a must. Large projects' use of Python 3.6 has shown that it's hard to track down the actual cause of the error; it only makes sense to improve that before the final release.

Serhiy, are you working on #28028 alongside this, or can it be removed from the dependencies?
msg279794 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 15:07
I agree it would be nice to get this in to 3.6. I'm not sure I'd go so far as to say it's a must and can't wait for 3.6.1. It's a non-trivial change, and it's up to Ned to say if it can go in to 3.6.

If you don't run with -Wall or -Werror, then you won't notice any new behavior with invalid escapes, correct?

Maybe post to python-dev and see if we can get more reviewers?
msg279795 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-10-31 15:10
I agree that the current behavior for 3.6 is very user-unfriendly so I think the risks of making such an extensive change at this point in the release cycle are outweighed by the severity of the problem.  So let's get it into 3.6 now; there's still time for it to make 360b3.
msg279796 - (view) Author: Anilyka Barry (abarry) * (Python triager) Date: 2016-10-31 15:18
Even better than what I was aiming for :)
msg279797 - (view) Author: Tim Graham (Tim.Graham) * Date: 2016-10-31 15:21
The patch is working well to identify warnings when running Django's test suite. Thanks!
msg279798 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 15:26
I'll work on this as soon as I can, coordinating with Ned.
msg279799 - (view) Author: Chih-Hsuan Yen (yan12125) * Date: 2016-10-31 15:31
The error message is much better now, thanks you all!

Seems the ^ pointer is not always correct. For example, in the function scope it's correct:

$ cat test.py 
def foo():
    s = 'C:\Program Files\Microsoft'

$ python3.7 -W error test.py
  File "test.py", line 2
    s = 'C:\Program Files\Microsoft'
           ^
SyntaxError: invalid escape sequence \P

On the other hand, top-level literals confuses the pointer:

$ cat test.py               
s = 'C:\Program Files\Microsoft'

$ python3.7 -W error test.py
  File "test.py", line 1
    s = 'C:\Program Files\Microsoft'
       ^
SyntaxError: invalid escape sequence \P

Is that expected?

Using 259745f9a1e4 on Arch Linux 64-bit
msg279820 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-10-31 18:49
New changeset ee82266ad35b by Eric V. Smith in branch '3.6':
Issue 28128: Print out better error/warning messages for invalid string escapes. Backport to 3.6.
https://hg.python.org/cpython/rev/ee82266ad35b

New changeset 7aa001a48120 by Eric V. Smith in branch 'default':
Issue 28128: Null merge with 3.6: already applied to default.
https://hg.python.org/cpython/rev/7aa001a48120
msg279821 - (view) Author: Eric V. Smith (eric.smith) * (Python committer) Date: 2016-10-31 19:26
Chi Hsuan Yen:

I'll investigate, and open another issue as needed.
History
Date User Action Args
2017-03-31 16:36:18dstufftsetpull_requests: + pull_request922
2016-10-31 19:26:57eric.smithsetstatus: open -> closed
resolution: fixed
messages: + msg279821

stage: commit review -> resolved
2016-10-31 18:49:13python-devsetmessages: + msg279820
2016-10-31 15:31:38yan12125setmessages: + msg279799
2016-10-31 15:26:06eric.smithsetmessages: + msg279798
2016-10-31 15:21:50Tim.Grahamsetmessages: + msg279797
2016-10-31 15:18:30abarrysetpriority: deferred blocker -> release blocker

dependencies: - Convert warnings to SyntaxWarning in parser
messages: + msg279796
2016-10-31 15:10:14ned.deilysetmessages: + msg279795
stage: patch review -> commit review
2016-10-31 15:07:01eric.smithsetmessages: + msg279794
2016-10-31 15:00:41abarrysetmessages: + msg279793
2016-10-31 13:33:44serhiy.storchakasetmessages: + msg279791
2016-10-31 13:23:02eric.smithsetmessages: + msg279790
2016-10-31 13:22:21python-devsetnosy: + python-dev
messages: + msg279789
2016-10-31 12:42:23eric.smithsetmessages: + msg279785
2016-10-31 12:36:07serhiy.storchakasetfiles: + 28128-7.diff

messages: + msg279784
2016-10-31 12:06:13eric.smithsetmessages: + msg279783
2016-10-31 12:02:43serhiy.storchakasetfiles: + 28128-6.diff

messages: + msg279782
2016-10-31 11:57:36eric.smithsetmessages: + msg279781
2016-10-31 10:39:30serhiy.storchakasetfiles: + 28128-5.diff

messages: + msg279778
2016-10-31 09:54:46serhiy.storchakasetfiles: + 28128-4.diff

messages: + msg279776
2016-10-31 08:43:43serhiy.storchakasetfiles: + 28128-3.diff
2016-10-31 08:36:02eric.smithsetmessages: + msg279771
2016-10-31 01:59:01abarrysetmessages: + msg279759
2016-10-31 01:17:37eric.smithsetfiles: + 28128-3.diff

messages: + msg279757
2016-10-31 00:58:37eric.smithsetstage: needs patch -> patch review
2016-10-31 00:58:28eric.smithsetfiles: + 28128-2.diff
2016-10-31 00:58:06eric.smithsetmessages: + msg279754
2016-10-30 17:31:48eric.smithsetassignee: eric.smith
2016-10-30 14:57:38eric.smithsetmessages: + msg279724
2016-10-29 06:54:29ezio.melottisetnosy: + ezio.melotti
2016-10-29 05:10:51ncoghlansetnosy: + petr.viktorin
messages: + msg279654
2016-10-29 03:46:58abarrysetpriority: normal -> deferred blocker

dependencies: + Convert warnings to SyntaxWarning in parser
messages: + msg279650
2016-10-07 16:00:31eric.smithsetmessages: + msg278251
2016-10-04 09:06:49serhiy.storchakalinkissue28354 superseder
2016-10-03 17:13:28ncoghlansetmessages: + msg277967
2016-10-03 17:06:35brett.cannonsetnosy: - brett.cannon
2016-10-02 23:48:13abarrysetmessages: + msg277916
2016-09-17 12:06:16ncoghlansetmessages: + msg276785
2016-09-16 18:04:10eric.smithsetmessages: + msg276731
2016-09-16 17:46:53serhiy.storchakasetmessages: + msg276730
2016-09-16 16:59:16Tim.Grahamsetmessages: + msg276728
2016-09-16 14:34:26abarrysetmessages: + msg276720
2016-09-16 14:25:40eric.smithsetmessages: + msg276717
2016-09-16 14:23:50eric.smithsetmessages: + msg276716
2016-09-16 14:22:12eric.smithsetfiles: + 28128.diff
keywords: + patch
messages: + msg276715
2016-09-16 11:35:15eric.smithsetmessages: + msg276698
2016-09-16 02:55:12ncoghlansetnosy: + eric.smith
messages: + msg276668
2016-09-16 02:26:50abarrysetmessages: + msg276666
2016-09-16 02:18:56ncoghlansetmessages: + msg276665
2016-09-16 02:05:58ncoghlansetmessages: + msg276663
2016-09-16 02:02:55yselivanovsetmessages: + msg276662
2016-09-16 02:01:07abarrysetmessages: + msg276661
2016-09-16 01:40:46ncoghlansetmessages: + msg276658
2016-09-16 00:13:42abarrysetnosy: + brett.cannon, rhettinger, ncoghlan, yselivanov
messages: + msg276646
2016-09-15 23:43:27Tim.Grahamsetnosy: + Tim.Graham
messages: + msg276641
2016-09-15 14:53:40r.david.murraysetmessages: + msg276560
2016-09-15 10:51:40abarrysetassignee: abarry -> (no value)
messages: + msg276548
2016-09-14 00:25:55martin.pantersetmessages: + msg276364
2016-09-13 20:45:04abarrysetnosy: + abarry
messages: + msg276341

assignee: abarry
components: - Library (Lib)
stage: needs patch
2016-09-13 19:04:42r.david.murraysetmessages: + msg276326
2016-09-13 15:43:12ned.deilysetmessages: + msg276293
2016-09-13 15:11:02yan12125setnosy: + ned.deily
messages: + msg276286
2016-09-13 15:10:24yan12125create