bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

orenmn · 2017-08-27T18:33:36Z

in _warnings.c: add checks whether the return value of the received loader's get_source() is invalid.
in test_warnings/__init__.py: add tests to verify the assertion failure and the SystemError are no more.

https://bugs.python.org/issue31285

…ource)

serhiy-storchaka · 2017-08-30T06:09:26Z

Python/_warnings.c

@@ -884,9 +884,9 @@ warnings_warn_explicit(PyObject *self, PyObject *args, PyObject *kwds)
        }

        /* Split the source into lines. */
-        source_list = PyObject_CallMethodObjArgs(source,
+        source_list = PyObject_CallMethodObjArgs((PyObject *)&PyUnicode_Type,


Use just PyUnicode_Splitlines().

serhiy-storchaka · 2017-08-30T06:24:11Z

Lib/test/test_warnings/__init__.py

+                            return splitlines_ret_val
+                    return BadSource()
+            return BadLoader()
+        self.assertRaises(IndexError, self.module.warn_explicit,


Since this test already takes multiple lines, I think it would look clearer if use a context manager form:

with self.assertRaises(IndexError): self.module.warn_explicit(...)

But why IndexError is expected? Wouldn't be better to test the normal case that doesn't raise an error? You can use test.support.captured_stderr() for capturing the warning output.

serhiy-storchaka · 2017-08-30T06:38:22Z

Lib/test/test_warnings/__init__.py

@@ -794,6 +794,33 @@ def test_stderr_none(self):
        self.assertNotIn(b'Warning!', stderr)
        self.assertNotIn(b'Error', stderr)

+    def test_warn_explicit(self):


This name is too general, but the test isn't a general test of warn_explicit(). Use something more specific like test_issue31285.

orenmn · 2017-08-30T12:38:49Z

Lib/test/test_warnings/__init__.py

+                'foo', UserWarning, 'bar', 1,
+                module_globals={'__loader__': _get_bad_loader(42),
+                                '__name__': 'foobar'})
+        self.assertIn('bar:1: UserWarning: foo', stderr.getvalue())


I am not sure whether this and the other call to assertIn() are necessary, as we want to make sure a SystemError and an assertion failure don't happen...

serhiy-storchaka · 2017-09-24T09:12:11Z

Lib/test/test_warnings/__init__.py

+        # warn_explicit() should neither raise a SystemError nor cause an
+        # assertion failure, in case the return value of get_source() has a
+        # bad splitlines() method.
+        def _get_bad_loader(splitlines_ret_val):


Starting underscore is not needed.

serhiy-storchaka · 2017-09-24T09:16:18Z

Lib/test/test_warnings/__init__.py

+            del self.module._showwarnmsg
+            with support.captured_stderr() as stderr:
+                self.module.warn_explicit(
+                    'foo', ArithmeticError, 'bar', 1,


Is it valid to use ArithmeticError which is not a warning category?

It is valid (IIUC, only warning.warns() requires the category to be a subclass of Warning). I used ArithmeticError because it caused get_filter() to return 'default'. But now it seems quite obscure to me, so I would change it.

Also, my test fails with -Werror, so I would fix that too.

miss-islington · 2017-09-24T18:27:14Z

Thanks @orenmn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6.
🐍🍒⛏🤖

miss-islington · 2017-09-24T18:27:18Z

Sorry, @orenmn and @serhiy-storchaka, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 91fb0afe181986b48abfc6092dcca912b39de51d 2.7

miss-islington · 2017-09-26T23:29:05Z

Thanks @orenmn for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 2.7, 3.6.
🐍🍒⛏🤖

miss-islington · 2017-09-26T23:29:06Z

Sorry @orenmn and @serhiy-storchaka, I had trouble checking out the 3.6 backport branch.
Please backport using cherry_picker on command line.
cherry_picker 91fb0afe181986b48abfc6092dcca912b39de51d 3.6

miss-islington · 2017-09-26T23:29:07Z

Sorry, @orenmn and @serhiy-storchaka, I could not cleanly backport this to 2.7 due to a conflict.
Please backport using cherry_picker on command line.
cherry_picker 91fb0afe181986b48abfc6092dcca912b39de51d 2.7

…gs.warn_explicit. (pythonGH-3219) (cherry picked from commit 91fb0af)

bedevere-bot · 2017-09-26T23:30:15Z

GH-3775 is a backport of this pull request to the 3.6 branch.

…gs.warn_explicit. (GH-3219) (#3775) (cherry picked from commit 91fb0af)

bedevere-bot · 2017-09-28T15:44:00Z

GH-3805 is a backport of this pull request to the 2.7 branch.

…pythonGH-3803) (forgot to remove it in pythonGH-3219) (cherry picked from commit 8b4ff53)

(forgot to remove it in #3219)

…GH-3803) (#3829) (forgot to remove it in GH-3219) (cherry picked from commit 8b4ff53)

…) in case __loader__.get_source() has a bad splitlines() method. (pythonGH-3219)

…) in case __loader__.get_source() has a bad splitlines() method. (GH-3219) (#3823) (cherry picked from commit 91fb0af)

orenmn added 2 commits August 27, 2017 20:18

init commit

ff52f6b

add a NEWS.d item

252647d

the-knights-who-say-ni added the CLA signed label Aug 27, 2017

bedevere-bot added the awaiting review label Aug 27, 2017

orenmn added 2 commits August 28, 2017 13:06

replace the C equivalent of source.splitlines() with str.splitlines(s…

d4dbe6d

…ource)

fix NEWS.d item

9eb55b5

serhiy-storchaka reviewed Aug 30, 2017

View reviewed changes

improve test method and its name, and change to PyUnicode_Splitlines()

5674174

orenmn commented Aug 30, 2017

View reviewed changes

bedevere-bot added awaiting core review and removed awaiting review labels Aug 30, 2017

resolve conflict in test_warnings/__init__.py

b8fe794

serhiy-storchaka reviewed Sep 24, 2017

View reviewed changes

orenmn added 3 commits September 24, 2017 14:52

merge master

64f31fc

remove redundant underscore

6fe73a4

fix the test, and make it clearer

71da7df

serhiy-storchaka added needs backport to 2.7 type-bug An unexpected behavior, bug, or error labels Sep 24, 2017

serhiy-storchaka approved these changes Sep 24, 2017

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting core review labels Sep 24, 2017

serhiy-storchaka merged commit 91fb0af into python:master Sep 24, 2017

serhiy-storchaka added needs backport to 3.6 and removed needs backport to 3.6 labels Sep 26, 2017

serhiy-storchaka pushed a commit to serhiy-storchaka/cpython that referenced this pull request Sep 26, 2017

[3.6] bpo-31285: Fix an assertion failure and a SystemError in warnin…

6430003

…gs.warn_explicit. (pythonGH-3219) (cherry picked from commit 91fb0af)

bedevere-bot removed the needs backport to 3.6 label Sep 26, 2017

serhiy-storchaka added a commit that referenced this pull request Sep 27, 2017

[3.6] bpo-31285: Fix an assertion failure and a SystemError in warnin…

90fe25a

…gs.warn_explicit. (GH-3219) (#3775) (cherry picked from commit 91fb0af)

orenmn mentioned this pull request Sep 28, 2017

bpo-31285: Remove splitlines identifier from Python/_warnings.c (forgot to remove it in #3219) #3803

Merged

bedevere-bot removed the needs backport to 2.7 label Sep 28, 2017

orenmn mentioned this pull request Sep 29, 2017

[2.7] bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit (GH-3219) #3805

Closed

miss-islington mentioned this pull request Sep 29, 2017

[3.6] bpo-31285: Remove splitlines identifier from Python/_warnings.c (GH-3803) #3829

Merged

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Sep 29, 2017

[3.6] bpo-31285: Remove splitlines identifier from Python/_warnings.c (…

3c3e62c

…pythonGH-3803) (forgot to remove it in pythonGH-3219) (cherry picked from commit 8b4ff53)

serhiy-storchaka pushed a commit that referenced this pull request Sep 29, 2017

bpo-31285: Remove splitlines identifier from Python/_warnings.c (#3803)

8b4ff53

(forgot to remove it in #3219)

serhiy-storchaka pushed a commit that referenced this pull request Sep 29, 2017

[3.6] bpo-31285: Remove splitlines identifier from Python/_warnings.c (…

66c2b9f

…GH-3803) (#3829) (forgot to remove it in GH-3219) (cherry picked from commit 8b4ff53)

orenmn added a commit to orenmn/cpython that referenced this pull request Sep 30, 2017

[2.7] bpo-31285: Don't raise a SystemError in warnings.warn_explicit(…

c15236b

…) in case __loader__.get_source() has a bad splitlines() method. (pythonGH-3219)

serhiy-storchaka pushed a commit that referenced this pull request Sep 30, 2017

[2.7] bpo-31285: Don't raise a SystemError in warnings.warn_explicit(…

40d736b

…) in case __loader__.get_source() has a bad splitlines() method. (GH-3219) (#3823) (cherry picked from commit 91fb0af)

Mariatta removed the awaiting merge label Oct 8, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

Uh oh!

orenmn commented Aug 27, 2017 •

edited by bedevere-bot

Loading

Uh oh!

serhiy-storchaka Aug 30, 2017

Uh oh!

serhiy-storchaka Aug 30, 2017

Uh oh!

serhiy-storchaka Aug 30, 2017

Uh oh!

orenmn Aug 30, 2017

Uh oh!

serhiy-storchaka Sep 24, 2017

Uh oh!

serhiy-storchaka Sep 24, 2017

Uh oh!

orenmn Sep 24, 2017

Uh oh!

miss-islington commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

bedevere-bot commented Sep 26, 2017

Uh oh!

bedevere-bot commented Sep 28, 2017

Uh oh!

Uh oh!

Uh oh!

bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

bpo-31285: fix an assertion failure and a SystemError in warnings.warn_explicit #3219

Uh oh!

Conversation

orenmn commented Aug 27, 2017 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

serhiy-storchaka Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

orenmn Aug 30, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

serhiy-storchaka Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

orenmn Sep 24, 2017

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 24, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

miss-islington commented Sep 26, 2017

Uh oh!

bedevere-bot commented Sep 26, 2017

Uh oh!

bedevere-bot commented Sep 28, 2017

Uh oh!

Uh oh!

orenmn commented Aug 27, 2017 •

edited by bedevere-bot

Loading